(not a stakeholder either)

On top of ignoring whitespace, Git and Github support ignoring commits when performing `git blame`.

With that tool, one commit could remove trailing whitespace over the all project and add some editorconfig or CI checks to prevent them from coming back. Then, a second commit would add the first commit to the ignore-revs-file.

https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt

https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

On 13 Apr 2023, at 08:43, Mathias Bynens via webkit-dev <webkit-dev@lists.webkit.org> wrote:

(not a stakeholder; just curious)

Does it help that Git supports the `-w` / `--ignore-all-space` flag to ignore whitespace changes? It works with `git diff`, `git blame`, etc.

GitHub also supports it — compare e.g.

https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec
vs.
https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec?w=1

On Wed, Apr 12, 2023 at 9:35 PM Chris Dumez via webkit-dev <webkit-dev@lists.webkit.org> wrote:


On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev <webkit-dev@lists.webkit.org> wrote:

Yeah, enforcing that new or otherwise modified lines don’t have trailing whitespaces would be good.

Yes, I wouldn’t mind that either.

However, https://commits.webkit.org/262879@main has just landed and if you look at the changes to Document.cpp, it is mostly spacing changes :(
It makes it harder to review or to identify meaningful changes in a patch after landing. It also pollutes git blame for no great reason.


- R. Niwa

On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki <ysuzuki@apple.com> wrote:

I agree that we should not do it because it pollutes change history of files, git-blame results, and review-diff in PR.
But at the same time, I think there is no reason to add a new trailing whitespace via a new commit.
It is nice if we can enforce this rule only for newly added code (via style-checker) not to add new trailing spaces.

-Yusuke

On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev <webkit-dev@lists.webkit.org> wrote:

WebKi proejctt’s long term policy has been to not do this:

I don’t think we should change that.

- R. Niwa

On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev <webkit-dev@lists.webkit.org> wrote:

I am against this because it adds a lot of noise to patches I am trying to review.
I have seen PRs where white space changes account for more than half the patch I am trying to review.

Dropping trailing spaces on the lines you’re modifying is OK but in the whole file is too noisy IMO.

Chris.

On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev <webkit-dev@lists.webkit.org> wrote:

To reduce the overhead of switching between projects with different
whitespace requirements, I would like to suggest we start being
lenient when trailing whitespace is removed. In particular when a file
is being changed to fix a bug.

I could see going even further and enforcing this via the style
checker, if there is appetite for that.

Thanks for considering!
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev
_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev