opened
Denton-L
username_0
When running `/submit` (or maybe even before if that's possible?) we should run `git diff --check` to warn users about whitespace issues. This would save some review cycles as reviewers wouldn't have to point out issues that can be automatically caught.

Note that this shouldn't block users from submitting patches. There exist a bunch of files (mostly tests and documentation) that contain existing whitespace errors so if they're making a change to the offending lines, it shouldn't prevent them from doing so.
Run `git diff --check` before submitting changes
issue
created
dscho
username_1
AFAICT `git diff --check` never complains about `-` or ` ` lines, only about `+` ones.
comment
created
Denton-L
username_0
I was thinking about the case where there is pre-existing trailing whitespace on a line. If a developer modifies the content of the line but keeps the whitespace, it would trigger the check. As a result, we should probably leave this as a warning and not an error.
comment
created
webstech
username_2
Running a check test was brought up in issue #82.
comment
created
dscho
username_1
Not sure. If it is an error, there should be a way to override it.

I'm just wary from a certain recent experience where a contributor got _two separate_ people comment on a problematic cover letter, and _still_ managed to completely ignore both comments and submit right away. In this instance, a comment from a bot would probably have been ignored even more completely, and I would want GitGitGadget to force a pause there.
comment
created
Denton-L
username_0
Running `git diff --check $(git hash-object -t tree /dev/null)`, there are a lot of matches. Although a lot of them are errors, it seems like a lot of them are intentional too. In that case, it wouldn't really make sense to error out since there's nothing to "fix". Perhaps this case will be pretty rare, though, in which case, it would make sense to error out and allow users to override if necessary.
comment
created
Denton-L
username_0
Also, more discussion on this here: https://lore.kernel.org/git/xmqqsgloj9fd.fsf@gitster-ct.c.googlers.com/

The suggestion is that we should add a Makefile target that automates this (and possibly others?) check. Then, GGG can call that target. This would benefit all developers, not just those on GGG.
comment
created
webstech
username_2
@username_1 What do you think about adding this to the push workflow in a new step?  When/if a makefile target becomes available, it can be changed to use it.  This could provide early notification of problems.
comment
created
dscho
username_1
Sure, good idea!
comment
created
webstech
username_2
Git PR #[709](https://github.com/gitgitgadget/git/pull/709) has been opened to run a check with each developer push.
comment
created
webstech
username_2
@username_0 Git PR [709](https://github.com/gitgitgadget/git/pull/709) has been closed.  The check will now be done when a pull is updated.  The implementation does not use a make target as suggested earlier in this thread.

Does the current change meet the needs of this issue (pending an actual makefile target)?
comment
created
Denton-L
username_0
@username_2 Thanks, meets the needs of the issue!
comment
closed
Denton-L
username_0
issue