On Wed, Apr 12, 2023 at 7:20 PM Yusuke Suzuki via webkit-dev
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.
(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
Besides enforcing no whitespaces in new lines. Are PRs exclusively removing
whitespaces from a file (or a batch of files) welcome?
> On 13. Apr 2023, at 12:20, Adrian Perez de Castro via webkit-dev
> wrote:
>
> Hi,
>
> On Wed, 12 Apr 2023 10:23:06 -0700 Ryosuke Niwa via webkit-dev
>
Hi,
On Wed, 12 Apr 2023 10:23:06 -0700 Ryosuke Niwa via webkit-dev
wrote:
> Yeah, enforcing that new or otherwise modified lines don’t have trailing
> whitespaces would be good.
I very much agree with this. Let's leave existing code as-is, having new and
modified lines without trailing
Hi,
On Wed, 12 Apr 2023 20:27:25 -0500 Michael Catanzaro via webkit-dev
wrote:
> On Thu, Apr 13 2023 at 08:15:00 AM +0900, Tetsuharu Ohzeki via
> webkit-dev wrote:
>
> > To digress a little, why does webkit now use a style checker based on
> > python script instead of clang-format?
> > In
(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
I checked the clang-format result of WTF.
$ find Source/WTF -name '*.h' -o -name '*.cpp' -exec clang-format -i '{}'
';'
Although it doesn't comply with the current WebKit style, it looks good
enough to me.
By adopting clang-format for the project, we can forget most parts of
WebKit style
On Thu, Apr 13 2023 at 08:15:00 AM +0900, Tetsuharu Ohzeki via
webkit-dev wrote:
To digress a little, why does webkit now use a style checker based on
python script instead of clang-format?
In today, I feel it's more reasonable to use such a formetter.
We've tried clang-format in several
To digress a little, why does webkit now use a style checker based on
python script instead of clang-format?
In today, I feel it's more reasonable to use such a formetter.
(This is just a off topic & just my interesting about history,
I don’t have a intention to propose in this discussion to
> On Apr 12, 2023, at 12:34 PM, Chris Dumez wrote:
>
>> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev
>> 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,
> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev
> 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
Yeah, enforcing that new or otherwise modified lines don’t have trailing
whitespaces would be good.
- R. Niwa
> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki wrote:
>
> I agree that we should not do it because it pollutes change history of files,
> git-blame results, and review-diff in PR.
>
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
WebKi proejctt’s long term policy has been to not do this:
https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
I don’t think we should change that.
- R. Niwa
> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev
> wrote:
>
> I am against this because it adds a lot of
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.
We have an .editorconfig in the repository already, we could/should take
advantage of it.
There has been a commit in 2020 to not enforce trailing whitespace removal,
but we can change it to enforce non-change of trailing whitespace.
How about adding the trim_trailing_whitespace option to the root's
.editorconfig as the first step?
- https://github.com/WebKit/WebKit/blob/main/.editorconfig
- https://editorconfig.org/
This does not enforce anything for all of us, so we still require some
style checker mechanism,
but I think
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
18 matches
Mail list logo