Re: [forge-dev] Moving forward with FORGE-183

plugin? Only in the SetupCommand, which is already done :) Not at the moment. For now, I would *not* worry about writing the Git plugins themselves. I would start with the UndoPlugin and the GitFacet functionality. We can worry about GitPlugin later, since we already have passthrough support for Git via the GitShellPlugin. Sensible? ~Lincoln On Tue, May 22, 2012 at 8:14 AM, Jevgeni Zelenkov < jevgeni.zelenkov(a)googlemail.com&gt; wrote: > Hi Forge developers, > > I am still stuck trying to get FORGE-183 pull request accepted. Did I > set the record already as the longest unaccepted pull request in the > Forge's history? :) > > Here is the summary of all the feedback I received so far: > > ---- > ---- > Does it make sense to require the Git Facet on the GitPlugin? Facets > imply that a project is also required, and it may be possible to use > GitPlugin outside of a project. This is a limitation, currently, where > Facet constraints cannot be placed on individual methods - but > something we hope to address in the future. So in the mean time, The > plugin would need to be split into two separate plugins - we can > discuss the best approach for this on the dev list. > -- > Regarding the GitFacet - I believe it's in the right place for the moment, > but I don't believe that the GitPlugin should be referencing it, since the > commands in GitPlugin could be used even without a Project. > > GitFacet (as the issue describes poorly) is something that should be usable > to inspect the state of a Project via: > > project.hasFacet(GitFacet.class) > > After that, everything else is an added benefit. It should probably also > perform tasks like: > > project.getFacet(GitFacet.class).getBranches() > project.getFacet(GitFacet.class).getTags() > project.getFacet(GitFacet.class).getRemotes() > project.getFacet(GitFacet.class).getStatus() > > And that's where your imagination kicks in. > -- > To confuse things even further, I would not implement the actual JGit > features in the Facet itself (even though it would be correct to do > so.) I would instead create a utility class that the Facet itself > would use. This utility class could be reused by plugins directly (for > plugins that do not necessarily operate on a project.) > > So, really FORGE-183 is open-ended, but in general it's meant to allow > projects to know when they contain a Git repository. You will need this > feature for the Git undo plugin. > -- > My recommendation is to implement GitFacet in such a way that it will be > "installed" or "registered" if the project contains, or exists in, a git > repository. Then once you have done that, I'd start working on the > UndoPlugin and UndoFacet. > ---- > ---- > > > Based on that feedback, I wrote a TODO list of updates, which should > be added to the current pull request to get it accepted: > > 1. remove "RequiresFacet" annotation from the Git plugin > 2. At least the following methods should be added to the Git plugin, > GitFacet and GitUtils: getBranches(), getTags(), getRemotes() and > getStatus(). GitFacet has no dependency on jGit, GitUtils is dependent > on jGit. GitFacet install() and isInstalled() methods are OK in their > current form. > 3. Current GitUtils methods should refactored, so that they don't > return any objects from jGit library, like Ref. (the only exception > being "public static Git git(final DirectoryResource dir)") > > Does that make sense? > > Here is a demo implementation of the getBranches methods for Git > plugin, GitFacet and GitUtils. Other methods will look similar. > > Git plugin method implementation: > > @Command("branches") > public void gitGetBranches() throws Exception > { > if (shell.getCurrentProject().hasFacet(GitFacet.class)) > shell.getCurrentProject().getFacet(GitFacet.class).getBranches(); > } > > > GitFacet method implementation: > > public List<String> getBranches() > { > Git git = GitUtils.git(project.getProjectRoot()); > return GitUtils.getBranches(git); > } > > > GitUtils method implementation (actually implements the functionality): > > public static List<String> getBranches(final Git repo) > { > List<String> results = new ArrayList<String>(); > > for(Ref branchRef : repo.branchList().call()) > { > results.add(branchRef.getName()); > } > > return results; > } > > > plugin? > > I would appreciate your feedback. > > > Best regards, > Jevgeni > _______________________________________________ > forge-dev mailing list > forge-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/forge-dev > -- Lincoln Baxter, III http://ocpsoft.org "Simpler is better."