[forge-dev] Moving forward with FORGE-183

[Lincoln Baxter, III]

> General questions:
> 1. Does installFacet() method needs to be called anywhere in the Git
plugin?

Only in the SetupCommand, which is already done :)

> 2. Does Git plugin should be split into two plugins? One containing
> project-related methods, and another, containing methods, which could
> be called outside of a project.

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 at googlemail.com> 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;
> }
>
>
> General questions:
> 1. Does installFacet() method needs to be called anywhere in the Git
> plugin?
> 2. Does Git plugin should be split into two plugins? One containing
> project-related methods, and another, containing methods, which could
> be called outside of a project.
>
> I would appreciate your feedback.
>
>
> Best regards,
> Jevgeni
> _______________________________________________
> forge-dev mailing list
> forge-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/forge-dev
>



-- 
Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/forge-dev/attachments/20120522/d2a69039/attachment.html