[webkit-reviews] review granted: [Bug 74679] Make it easier to add files to XYZ.xcodeproj/project.pbxproj with the commit-queue : [Attachment 121212] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Benjamin Poulain
<benjamin at webkit.org>'s request for review:
Bug 74679: Make it easier to add files to XYZ.xcodeproj/project.pbxproj with
the commit-queue
https://bugs.webkit.org/show_bug.cgi?id=74679

Attachment 121212: Patch
https://bugs.webkit.org/attachment.cgi?id=121212&action=review

------- Additional Comments from David Kilzer (:ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=121212&action=review


r=me, but consider altering the test to make it clear which section is causing
the error.

This is a great step in the right direction.  All we need to do is to sort the
Xcode project files for these checks to be effective!

> Tools/Scripts/webkitpy/style/checkers/xcodeproj.py:76
> +	   If the current match should not be after the previous match, issue
and style error."""

Grammar:  "issue and style error."?  Maybe "issue a style error."?

> Tools/Scripts/webkitpy/style/checkers/xcodeproj.py:133
> +			       # FIXME: Use the exact same rules as
sort-Xcode-project-file.

Should probably file a bug for this and include it in the comment.

> Tools/Scripts/webkitpy/style/checkers/xcodeproj_unittest.py:122
> +	   self.assert_error(['developmentRegion = English;',
> +				 'mainGroup = 00B9318013BA867F0035A948 /*
WebKit */;',
> +				 '00B9318013BA867F0035A948 /* WebKit */ = {',
> +				 '    isa = PBXGroup;',
> +				 '    children = (',
> +				 '	  00A629C013D0BEC70050AC52 /*
MarkupTokenBase.h */',
> +				 '	  00022E6813CE1BBA00282D5B /*
CharacterReferenceParserInlineMethods.h */,',
> +				 '    );',
> +				 '}',
> +				 'children = (',
> +				 '00A629C013D0BEC70050AC52 /* MarkupTokenBase.h
*/',
> +				 '00022E6813CE1BBA00282D5B /*
CharacterReferenceParserInlineMethods.h */,',
> +				 ');'], 'Resource files not sorted
alphabetically: "CharacterReferenceParserInlineMethods.h" <
"MarkupTokenBase.h".')

Nit: Might be less confusing to have different hashes/filenames in the outer
'children' section so that you know which one is causing the error.