Summary: | generate-xcfilelists is stranding temporary files | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keith Rollin <krollin> | ||||||
Component: | Tools / Tests | Assignee: | Keith Rollin <krollin> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, glenn, jbedard, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Keith Rollin
2019-05-17 16:46:47 PDT
Created attachment 370169 [details]
Patch
Attachment 370169 [details] did not pass style-queue:
ERROR: Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:368: expected 1 blank line, found 0 [pep8/E301] [5]
Total errors found: 1 in 2 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 370169 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370169&action=review > Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:346 > + return set([re.sub(to_replace, replace_with, line) for line in lines]) Putting these in a set will throw out any order you have, not sure if that's an OK side-effect here....it might not have correctness problems, but I could see it making the xcfilelists less readable. > Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:368 > + def get_lines(source): The style checker is complaining about no newlines surrounding this. Given how you're using it, I wonder if this: get_lines = lambda source: source if isinstance(source, set) else set(source) if isinstance(source, list) else self._get_file_lines(source) would be more readable (and make the style checker happy) (In reply to Jonathan Bedard from comment #3) > Comment on attachment 370169 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=370169&action=review > > > Tools/Scripts/webkitpy/generate_xcfilelists_lib/generators.py:346 > > + return set([re.sub(to_replace, replace_with, line) for line in lines]) > > Putting these in a set will throw out any order you have, not sure if that's > an OK side-effect here....it might not have correctness problems, but I > could see it making the xcfilelists less readable. Actually, what we probably want to do is put everything in a set, then convert the set to a list, then sort the list. > ... (In reply to Jonathan Bedard from comment #4) > Actually, what we probably want to do is put everything in a set, then > convert the set to a list, then sort the list. That's exactly what I do. See _merge_added_lines. Created attachment 370257 [details]
Patch
Added blank lines. Comment on attachment 370257 [details] Patch Clearing flags on attachment: 370257 Committed r245531: <https://trac.webkit.org/changeset/245531> All reviewed patches have been landed. Closing bug. |