Address some of the more obvious linter warnings.
Created attachment 416826 [details] proposed patch
<rdar://problem/72782271>
View in context: https://bugs.webkit.org/attachment.cgi?id=416826&action=review rs=me > Tools/Scripts/webkitpy/layout_tests/servers/apache_http_server.py:158 > test_dir: absolute path to the LayoutTests directory. Should delete this line as well. > Tools/Scripts/webkitpy/tool/commands/upload.py:-511 > - bug_id = tool.bugs.create_bug(bug_title, comment_text, options.component, diff, "Patch", cc=options.cc, mark_for_review=options.review, mark_for_commit_queue=options.request_commit) Maybe we could add a log statement here to log the bug_id (either user visible or at DEBUG level). Not sure how helpful that would be.
Comment on attachment 416826 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=416826&action=review I don't think we should be changing some of the style checker ones without just removing the classes in question > Tools/Scripts/webkitpy/style/checkers/jsonchecker.py:34 > + def __init__(self, handle_style_error): If this class isn't accepting a file path, what is it even doing? Seems like the better approach on this one would be to make it actually work, which I think would be be done by reading the file in, then printing the json with an indent of 4 and comparing. > Tools/Scripts/webkitpy/style/checkers/watchlist.py:40 > + def __init__(self, handle_style_error): If this class isn't accepting a file path, what is it even doing? > Tools/Scripts/webkitpy/style/checkers/xml.py:34 > self._handle_style_error = handle_style_error If this class isn't accepting a file_path, what is it even doing? > Tools/Scripts/webkitpy/tool/steps/suggestreviewers_unittest.py:48 > + OutputCapture() Should just remove this entirely.
> If this class isn't accepting a file path, what is it even doing? These checkers use the lines argument, and don't need a file path. There can be an argument for keeping a uniform interface for all checkers though. I'm going to revert this part for now, because it's also incorrect - JSONChecker has subclasses that are still created with a file path argument, and are thus completely broken by this patch. And there are zero unit tests for any of them, so test-webkitpy didn't tell me about the mistake.
Committed https://trac.webkit.org/r271158
This broke `run-builtins-generator-tests`: Traceback (most recent call last): File "./Tools/Scripts/run-builtins-generator-tests", line 48, in <module> sys.exit(main(sys.argv)) File "./Tools/Scripts/run-builtins-generator-tests", line 45, in main return BuiltinsGeneratorTests(reset_results, executive.Executive()).main() File "/Volumes/Data/slave/catalina-release-tests-wk2/build/Tools/Scripts/webkitpy/codegen/main.py", line 161, in main if not self.run_tests(input_directory, reference_directory): File "/Volumes/Data/slave/catalina-release-tests-wk2/build/Tools/Scripts/webkitpy/codegen/main.py", line 148, in run_tests if not self.run_test(reference_directory, "WebCoreJSBuiltins.h", separately_generated_files, self.wrappers_builtin_test): File "/Volumes/Data/slave/catalina-release-tests-wk2/build/Tools/Scripts/webkitpy/codegen/main.py", line 114, in run_test if generate_builtin_callback(test_name, test_files, work_directory): TypeError: wrappers_builtin_test() takes exactly 3 arguments (4 given) https://build.webkit.org/builders/Apple-Catalina-Release-WK2-Tests/builds/10897/steps/builtins-generator-tests/logs/stdio
(In reply to Ryan Haddad from comment #7) > This broke `run-builtins-generator-tests` Reverted the relevant part of r271158 in r271173.