awslabs / damo

DAMON user-space tool
https://damonitor.github.io/
GNU General Public License v2.0
148 stars 28 forks source link

pre-commit: Add yapf code formatter hook #63

Closed honggyukim closed 12 months ago

honggyukim commented 1 year ago

yapf[1] is a powerful python code formatter and this patch helps the code base consistent with its pre-commit hook automatically.

The line breaking rule is set to 80 characters in a single line at pyproject.toml.

[1] https://github.com/google/yapf

honggyukim commented 1 year ago

Hi SeongJae,

This is another python code formatting tool, but might be more suitable for the current damo coding style. Please have a look and let me know if you have something that you don't like.

Thanks.

honggyukim commented 1 year ago

You can learn more about the difference between yapf and black from https://safjan.com/black-vs-yapf.

Some highlights from the article are as follows.

  1. Formatting rules Black enforces a strict set of formatting rules that are non-configurable. The aim of these rules is to provide consistent code formatting and reduce the time spent on code review. Black aims to minimize the differences in formatting style among developers, which can cause confusion when working on large projects. Yapf, on the other hand, allows users to customize the formatting rules according to their preferences. Yapf provides a set of default rules, but users can modify them by specifying options in the configuration file.

  2. Speed Black is known for its speed and can format code in a matter of seconds. Black achieves this speed by using a simple algorithm that focuses on making the smallest possible changes to the code. Yapf, on the other hand, is slower than Black but still relatively fast. Yapf's algorithm is more complex and focuses on making more significant changes to the code.

honggyukim commented 12 months ago

Hi SeongJae, I don't insist this tool should be used, but just give you some ideas what it can do.

sj-aws commented 12 months ago

Sorry for late response. IMHO, I think it's a little early or late to pick this at the moment. Let me explain why I think so in detail.

I like strict formatting rules, and try to respect those. One reason why I like Golang is it's language-supporting formatting rule and the tool. Similarly I love Python's strict indentation rule. Nevertheless, I know each person can have their own taste, and I want to keep those as long as it doesn't harm the consistency, and respect the agreed rule.

In case of damo, we don't have our own rule yet. Hence, my current humble mindset for the code formatting is, striving to only respect the Python's indentation rule while keeping the code look not that inconsistent. Of course that's not ideal.

In ideal, I think we should have our own well-discussed and well-defined rule. And then, we could adopt a formatting tool respecting our own rule. Of course, we could borrow a rule from a tool. But, I'd prefer to discuss the detail of each of the rule, and adopt each rule by modifying the code, for one by one. Only after that, I'd like to adopt the formatting tool. In other words, I want adopting of a formatting tool is not making huge code change.

At the moment, I don't understand the each rule of the formatting tool. And unfortunately I would have no enough time to read the documentation of the tool, at least until the OSSummit EU talk, because I aim to add more features that will be officially supported until then, and announce on the conference.

So sorry for saying this to your great effort, but I hope I explained how I think about this change at the moment.

honggyukim commented 12 months ago

I totally understand what you mean. I didn't want to force you to accept this strict coding style rule because I also see that python formatting tools do not have enough flexibility as much as I thought.

Although this is not accept now, but it would be worth to have this history so that we can revisit this formatting issue years after and learn what we have discussed here.

Thanks very much for your kind explanation!