Re: [egit-dev] Commit Validation

[Matthias Sohn <matthias.sohn@xxxxxxxxx>]

On Fri, Mar 13, 2015 at 1:02 AM, Andrey Loskutov <loskutov@xxxxxx> wrote:

Few notes:

 

1) IMHO Egit should not create markers since it does not listen to the resource modificatons (is the problem fixed?) and also does not contribute builders to the projects.

I think that's not true, we listen to resource modifications which drive incremental IndexDiff updates, that's actually what keeps staging view up-to-date.

Which problem do you mean ?

So if EGit validators listen on IndexDiff changes like staging view does I think we have a nice way to trigger validation. This would only be done

on resources tracked in a git repository shared with the EGit team provider. If e.g. a project is disconnected from EGit we should clear the

respective problem markers.

 

The problem with the markers is that user expects that they disappear after the problem is resolved or after "clean project", but if there is no egit listener/builder on the project, they will remain. We could add such a builder, but this is going out of the original proposal's scope (the builder will need to run pre-commit validators over each changed file, so that user could see the warnings even *before* commiting the stuff). This is interesting idea and deserves an extra discussion. Just imagine we would see all the "trailing white space" errors before they will appear in the gerrit. 

yes, exactly I want to show problems as soon as possible. E.g. if the user saves a file tracked by EGit which contains trailing whitespace the corresponding indexdiff would change, an EGit validator could recognize there's trailing whitespace and create a problem marker. As soon

as the user fixed this and saved the affected file the indexdiff would change again and EGit would revalidate the file and remove the problem

marker if validation is ok.

Another thing we could get from integration with problem markers is that it gives us a way to enable validations based on

problem markers created by builders which don't need to be integrated directly with EGit. E.g. we could configure EGit to

validate that only compilable code can be committed. So we could show a warning on commit "There are compile errors xyz

do you really want to commit this changes ?" Or "you have compiler warnings abc".

 

What we can initially do is just to run validators each time an entry appears in the staging view and also decorate entries in the staging view with extra information (all icons/font/text labels are possible), and even eventually disabling the commit button while showing "commit validation rule error on file xyz" error description in the staging view.
 

2) Matthias, me and Christian - we all understand this validation as "pre-commit" thing, which *prevents* commit to happen if something is not OK. Markus proposal works *after the fact*, and this is an important difference. From the user perspective it does really not make sense to fiddle with "bad" commit, it is not user friendly. I guess no one of my coworkers ever ameded a commit or tried to modify the history in any way. So if implementing validators, they must be *pre*-commit/pre-push etc.

yes, I think we should raise problems as soon as possible, e.g. if you save a file with trailing whitespace I think it's better to be informed

about that immediately than to defer this until commit. 

-Matthias