[forge-dev] Moving forward with FORGE-183

[Jevgeni Zelenkov]

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