Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Commit Validation

In this case, maybe multiple points should be hookable – before and after the commit has been done. My only concern here is the “how/where to register hooks”, and the “what should hooks return, and how to present it”.

 

We need JGit’s help (definitely) to be able to get called during the commit process, at the same locations as other hooks. But do we register each validator directly with JGit (how?), or only register EGit hooks with JGit somehow (API?), and EGit distributes to individual validators registered through ... an extension point? ... or some API?

 

What should these validators return, and what is their input? I opt for IStatus as return (can be nested, provides possibility to transfer all required information). For the input, at least the repository, but in some hooks there is more information available/required (commit-msg: the message, post-commit: the commit object), so it is a little hard to find a single generic interface for that... do you agree?

 

Cheers,

Markus

 

Von: egit-dev-bounces@xxxxxxxxxxx [mailto:egit-dev-bounces@xxxxxxxxxxx] Im Auftrag von Laurent Delaigue
Gesendet: Freitag, 13. März 2015 12:05
An: Andrey Loskutov; Matthias Sohn
Cc: EGit developer discussion (egit-dev@xxxxxxxxxxx)
Betreff: Re: [egit-dev] Commit Validation

 

Hi,

I agree with Andrey that EGit should not create markers.
I think it could be interesting to allow JGit to notify listeners at particular moments in time (specifically, in the places where hooks are called) to give the opportunity to clients to register listeners to react. I've been working on hooks lately and I think this should be quite easy.
This would allow java listeners to be registered and react at pre-commit, commit-msg, or any other step, with the same side-effects as hooks have (especially in terms of being able to cancel an operation).

I don't agree that validations should be pre-commit, git allows some many different workflows that we should support this flebility: if users want to be warned after a commit is made, I think it's fine.

Best regards,

Laurent Delaigue (@laurentdelaigue)
Obeo - Model-Driven Company

 

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.

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.

 

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.

 

Kind regards,
Andrey Loskutov

http://google.com/+AndreyLoskutov

 

 

Gesendet: Donnerstag, 12. März 2015 um 17:48 Uhr
Von: "Matthias Sohn" <matthias.sohn@xxxxxxxxx>
An: "Duft Markus" <Markus.Duft@xxxxxxxxxxxxxxxx>
Cc: "Halstrick, Christian" <christian.halstrick@xxxxxxx>, "Andrey Loskutov" <loskutov@xxxxxx>, "EGit developer discussion (egit-dev@xxxxxxxxxxx)" <egit-dev@xxxxxxxxxxx>
Betreff: Re: [egit-dev] Commit Validation

On Thu, Mar 12, 2015 at 7:27 AM, Duft Markus <Markus.Duft@xxxxxxxxxxxxxxxx> wrote:

Hey all,

It's gotten quite silent about this. Could somebody give any feedback on the change? I'm OK with "we won't accept it this way" also ;) (maybe as long as there is a viable alternative to do something similar in quality for the end-user).

Just as a demo on how I imagine validators based on the change (real world examples): https://drive.google.com/folderview?id=0B05h8C3gLt38MVlvOFFaUlh4Yk0&usp=sharing 

 

I appreciate that you are looking into more powerful validation.

I think it makes sense to provide more powerful validations which are better integrated into 

the Eclipse workbench than what can be done in scripted hooks.

 

I think we should go one step farther than what you propose.

I wonder if validators should create problem markers [1] and the git commands like

the commit command should check if problems of certain types/severity exist in order

to decide if it's ok to create a commit or if just to show there are problems.

 

Which problem types/severities should be reported and/or block git commands should be configurable.

 

This way we would gain an integration with other validators like build, compiler warnings etc

without imposing an EGit specific API for integrating them.

 

So you could e.g. configure that a warning should be shown in the commit dialog/staging view

if there are compiler warnings of a certain kind. Or commit could be blocked if the code doesn't

compile. We could add new validators for git specific validations like a commit message validator

checking if formatting of the commit message is ok, contains a link to a bug etc.

 

Validations could be also integrated with other git commands, e.g. if you want to prevent

that large blobs are entering the git repository the best place to block that would be the AddCommand

since as soon as you staged the big file it's already in the object store of your git repository.

 

We could use the problem view to display the problems and even implement quick fixes for

git specific new problem types.

 

 

-Matthias 




_______________________________________________
egit-dev mailing list
egit-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/egit-dev

 

Salomon Automation GmbH | Friesachstrasse 15 | 8114 Friesach bei Graz | Austria
Registered Office: Friesach bei Graz | Commercial Register: 49324 K | VAT no. ATU28654300
Commercial Court: Landesgericht für Zivilrechtssachen Graz

Back to the top