Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Conversation

@antis81
Copy link
Member

@antis81 antis81 commented Mar 7, 2015

Some thoughts about this PR:
It is not a good idea to realize the remotes menu in a central place. Why? Because from a users point of view there is no obvious context (What am I fetching/pushing from/to where? To which repository will a new remote belong?). Also the options to the fetch process are context-specific. The actions should actually be added to the appropriate context menus. For example a "fetch action" on a remote branch, fetches the remote branch. IOW: This belongs to the Reference-Tree and probably to the inline remote references in the History-View (we don't have a context menu for them yet). The actions itself should be defined central in the "Remotes" module, which should provide a the generic remote operations and just needs to be attached to the appropriate context. For this reason, the concept of the DAM (Dynamic Action Merger) has to be fully implemented first. This PR has to be hold back until then.

This is how it could look like?
mgv-fetch-repo-ctx-menu

Planned for this PR:

  • Fetch all remotes.
  • Fetch a single remote (only when more that one remote are configured)
    📝 Another PR will be created for this after integrating the MacGitverModules submodule.

Excluded from this PR

  • Move actions from "Remotes" application menu to the appropriate context menus.
  • Fetch a single branch.
    This is planned as an action for the "Reference-Tree" (and the inline references in "History-View"). See ticket on Redmine

More Ideas:

  • Prune remotely deleted branches (aka git fetch --prune).
    One idea is, to provide a "Cleanup" context menu to the repository tree, with options "Prune Branches", "Garbage Collection (gc)", ...
  • add a flag or option to recurse into submodules -> this clashes somewhat with the git config.fetch.recurseSubmodules flag.

@antis81 antis81 force-pushed the ngf/repo-fetch-remote branch from 9705062 to e0c8ecc Compare March 16, 2015 12:10
@antis81
Copy link
Member Author

antis81 commented Mar 16, 2015

@scunz This is ready to merge.

EDIT: FYI. To make life easier, I moved this PR from sc/refsview back to development. It will not compile, due to the small changes, that went to the wrong place. I don't care and move on anyways.

@scunz
Copy link
Member

scunz commented Mar 16, 2015

No, this is not ready to merge. Please hold off until I finished my mail - then you'll understand why - but it's taking ages to do that :)

@antis81
Copy link
Member Author

antis81 commented Mar 16, 2015

Yeah, I know this is unfinished stuff. I just want to trigger the merge as soon as possible, to help getting the submodule integration done quickly, before this gets into a more complex discussion and delaying things until next year - ya know what I mean :).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we add a return; here, we could merge this code now and activate it when we're there...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this code does what it should do. Why should we remove that? 😄

In this special case, the action is actually working and we add value by adding the unfinished fetch operation. The only thing is: A "fetch" action currently works just with unencrypted protocols and it doesn't show progress yet.

⚡ Returning a function without doing anything is the worst thing a programmer could code in released code. How would you react to buttons without a function? For an example, see the "Remotes" menu 😄.

Btw.: How about providing a GW_TODO macro in libGitWrap, that produces an error Git::Result with error code GIT_EUSER and a message like this:

The method 'Git::BestOperationEver::run()' in file:line is not implemented yet. Want to help out? Please contact us: <-maintainer->@macgitver.org.

Inspirational credits go to Blender 😄.

antis81 added 6 commits March 18, 2015 10:54
This is meant as a POC and not a full working implementation. The general idea is to create a threaded background-operation for each remote of the selected repository.

Requires also extensions/completions in the GitWrap API (i.e. to set the remote-alias (i.e. "origin").
@antis81 antis81 force-pushed the ngf/repo-fetch-remote branch from e0c8ecc to 86ed2d7 Compare March 18, 2015 09:59
@antis81
Copy link
Member Author

antis81 commented Mar 20, 2015

Closing, as this is a POC, not a final implementation. Moved to new PR #104 taking a different approach to abstract remote operations in a separate MacGitver module.

EDIT: I'll keep the branch to not lose the code.

@antis81 antis81 closed this Mar 20, 2015
@antis81 antis81 deleted the ngf/repo-fetch-remote branch March 20, 2015 07:53
@antis81 antis81 restored the ngf/repo-fetch-remote branch March 20, 2015 07:53
@antis81 antis81 deleted the ngf/repo-fetch-remote branch March 21, 2015 06:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants