Skip to main content

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

Validators should not create markers, but instead be able to use existing marker types to do the validation. In a second step EGit can also add its own markers but they should really be EGit specific, e.g. I would not expect EGit to create markers for trailing whitespace.

Dani



From:        Matthias Sohn <matthias.sohn@xxxxxxxxx>
To:        Duft Markus <Markus.Duft@xxxxxxxxxxxxxxxx>
Cc:        "EGit developer discussion \(egit-dev@xxxxxxxxxxx\)" <egit-dev@xxxxxxxxxxx>
Date:        16.03.2015 13:14
Subject:        Re: [egit-dev] Commit Validation
Sent by:        egit-dev-bounces@xxxxxxxxxxx




I think validators should just create markers and EGit should allow to configure which markers shall raise warnings
or block command execution when jgit commands are invoked.

We could start by first adding a way to configure which markers should raise a warning or block commit
when commit operation is started. This could be tested e.g. using compiler warnings or compile errors.

Then we can incrementally add more validations to create other markers and enhance configuration
to enable intercepting additional commands, e.g. "Add to Index".

-Matthias

On Mon, Mar 16, 2015 at 11:02 AM, Duft Markus <Markus.Duft@xxxxxxxxxxxxxxxx> wrote:
Hey,

 

OK, Markers might have some advantages, and I like the possiblities that open up there. However we still do need a vast amount of hook points in EGit (and JGit) to allow validation of different things in different locations – for example the commit message; how would that fit into this kind of infrastructure? Would validation have the possiblity to abort a commit (like jgit hooks)? Or does it “only” create markers and EGit “listens” to certain categories of them (to not even start the commit process)...?

 

Cheers,

Markus

 

Von: Matthias Sohn [mailto:matthias.sohn@xxxxxxxxx]
Gesendet:
Freitag, 13. März 2015 18:05
An:
Andrey Loskutov
Cc:
Duft Markus; Halstrick, Christian; EGit developer discussion (
egit-dev@xxxxxxxxxxx)
Betreff:
Re: Re: [egit-dev] Commit Validation

 

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

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

_______________________________________________
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


Back to the top