bneradt commented on code in PR #10860: URL: https://github.com/apache/trafficserver/pull/10860#discussion_r1410085440
########## .style.yapf: ########## @@ -0,0 +1,421 @@ +####################### +# +# Licensed to the Apache Software Foundation (ASF) under one or more contributor license +# agreements. See the NOTICE file distributed with this work for additional information regarding +# copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License +# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express +# or implied. See the License for the specific language governing permissions and limitations under +# the License. +# +####################### + +[style] +based_on_style = yapf + +# Align closing bracket with visual indentation. +align_closing_bracket_with_visual_indent=False + +# Allow dictionary keys to exist on multiple lines. For example: +# +# x = { +# ('this is the first element of a tuple', +# 'this is the second element of a tuple'): +# value, +# } +allow_multiline_dictionary_keys=True + +# Allow lambdas to be formatted on more than one line. +allow_multiline_lambdas=False + +# Allow splitting before a default / named assignment in an argument list. +allow_split_before_default_or_named_assigns=False + +# Allow splits before the dictionary value. +allow_split_before_dict_value=False + +# Let spacing indicate operator precedence. For example: +# +# a = 1 * 2 + 3 / 4 +# b = 1 / 2 - 3 * 4 +# c = (1 + 2) * (3 - 4) +# d = (1 - 2) / (3 + 4) +# e = 1 * 2 - 3 +# f = 1 + 2 + 3 + 4 +# +# will be formatted as follows to indicate precedence: +# +# a = 1*2 + 3/4 +# b = 1/2 - 3*4 +# c = (1+2) * (3-4) +# d = (1-2) / (3+4) +# e = 1*2 - 3 +# f = 1 + 2 + 3 + 4 +# +arithmetic_precedence_indication=False + +# Number of blank lines surrounding top-level function and class +# definitions. +blank_lines_around_top_level_definition=2 + +# Number of blank lines between top-level imports and variable +# definitions. +blank_lines_between_top_level_imports_and_variables=1 + +# Insert a blank line before a class-level docstring. +blank_line_before_class_docstring=False + +# Insert a blank line before a module docstring. +blank_line_before_module_docstring=False + +# Insert a blank line before a 'def' or 'class' immediately nested +# within another 'def' or 'class'. For example: +# +# class Foo: +# # <------ this blank line +# def method(): +# pass +blank_line_before_nested_class_or_def=True + +# Do not split consecutive brackets. Only relevant when +# dedent_closing_brackets is set. For example: +# +# call_func_that_takes_a_dict( +# { +# 'key1': 'value1', +# 'key2': 'value2', +# } +# ) +# +# would reformat to: +# +# call_func_that_takes_a_dict({ +# 'key1': 'value1', +# 'key2': 'value2', +# }) +coalesce_brackets=False + +# The column limit. +column_limit=99 Review Comment: I initially tried 132 locally, but I think the diff is actually larger if we set the line length to that. In reality, if you look at our .py tests before this patch, the community has kind of implicitly respected about a 80-100 character limit while autpep8 didn't "expand" things to 132 characters. yapf will actively expand function parameters and arguments and the like to 132 if we set that size. I'm not opposed to setting 132, I'm just stating it didn't seem like using 132 reduced the patch size but rather increased it. I can switch things to 132 if we like. The reason the patch is so large is because of the three big Python formatters (yapf, autopep8, black), autopep8 is the least "opinionated". yapf enforces much more style over autopep8. It enforces things at the level more akin to clang-format, for instance. I'm not sure that's bad, but it is how it is. I guess the big question is whether we think the changes are a net benefit or not. I feel like we kind of need to move off of autopep8 because I've lost confidence in the project - 6 weeks after an issue was filed about Python 3.12 and there's not even so much as a comment on the issue from any maintainers. Not even when I explicitly (and politely) asked for status with an @ mention of the repo author have they replied after 8 days. I intend, if I have time, to create a separate patch seeing how things look with the black formatter so we can compare. yapf has a pretty strong community which is a big plus, so I'll first investigate who and how much black is supported before trying them. Like all formatters, the goal is to find something "acceptable". "Beyond awesome" is probably not attainable. Anyway, thanks for looking at the patch. Feel free to provide other thoughts. Or even to try things out locally, if you like. It's pretty easy to experiment by using my first to commits in this PR. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
