Bug 63799 – check-webkit-style might better flag unsorted files enties in project.pbxproj

• Component: Tools / Tests
• Status: RESOLVED INVALID

[Kenichi Ishibashi]

We should run Tools/Scripts/sort-Xcode-project-file when adding files in project.pbxproj.

[Kenichi Ishibashi]

Created attachment 99457 [details]
Patch V0

[Mark Rowe (bdash)]

I don’t think it’s particularly useful to require that the Xcode file be resorted after every single change.  Is there some reason that you think it would be beneficial to do this?

[Kenichi Ishibashi]

Hi Mark,

Thank you for a comment.

(In reply to comment #2)
> I don’t think it’s particularly useful to require that the Xcode file be resorted after every single change.  Is there some reason that you think it would be beneficial to do this?

I usually run sort-Xcode-project-file when I add new files and upload a patch to review.  If a project.pbxproj isn't sorted in advance, my patch includes irrelevant stuff and I just don't want including them.   In addition, I sometimes need to resolve a conflict regarding this problem during review process, which is not needed in essence.  Anyway, if we need not sort them when we land a patch, I don't stick it.  Some reviewers recommended to sort them when I  sent  a patch to review, though.

[Kent Tamura]

I recommend to sort it in reviews in order to avoid merge conflicts.

[David Levin]

I have mixed feelings about this.
I think the sorting is recommended however I don't believe many reviewers flag it, and I don't know if we should nit it really.

Perhaps this deserves a quick note to webkit-dev to get a feeling on whether folks think this is something that we should be enforcing.

[Eric Seidel (no email)]

Well, if it's sorted after every change, then it's never out of sorts. :)  I'm not sure how much this is buying us though.

[Eric Seidel (no email)]

Comment on attachment 99457 [details]
Patch V0

View in context: https://bugs.webkit.org/attachment.cgi?id=99457&action=review

> Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py:94
> +                          'SurrogatePairAwareTextIterator.cpp should be alphabetically sorted.')

This should suggest using sort-xcode-proj, otherwise people won't know what to do.

[Kenichi Ishibashi]

Created attachment 127755 [details]
Patch

[Kenichi Ishibashi]

Comment on attachment 99457 [details]
Patch V0

View in context: https://bugs.webkit.org/attachment.cgi?id=99457&action=review

Thank you for comment. Revised the patch (but I'm not sure whether everyone can agree with doing this.)

>> Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py:94
>> +                          'SurrogatePairAwareTextIterator.cpp should be alphabetically sorted.')
> 
> This should suggest using sort-xcode-proj, otherwise people won't know what to do.

Done.

[David Levin]

fwiw, I don't object. (If folks want to review it and get it landed, then if there are problems it could be re-evaluated.)

[Anders Carlsson]

Comment on attachment 127755 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.