Ancestors

Written by Jens Axboe on 2024-09-12 at 18:18

Having worked on the kernel for decades, and imposing a lot of the same code/git hygiene for liburing, there can be a disconnect for contributors on what is expected of a commit and commit message, and what series of commits should look like. I attempted to provide a basic guideline here:

https://github.com/axboe/liburing/blob/master/CONTRIBUTING.md

and would appreciate feedback from folks on what I missed, what isn't clear, etc.

=> More informations about this toot | More toots from axboe@fosstodon.org

Written by Anisse on 2024-09-12 at 18:23

@axboe do you have any strategy for ensuring bisectability while also being on GitHub? A quick look at your workflow shows it does not build&test every commit, only PRs. Do you do that yourself locally?

=> More informations about this toot | More toots from Aissen@treehouse.systems

Written by Markus Werle on 2024-09-13 at 06:44

@Aissen @axboe the paragraph about coding style leaves me with a vague appeal. What does this really mean? Do you have a .clang-format file I can use to bring my code into shape? What else do you mean with coding style? Something beyond formatting and variable naming conventions?

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Markus Werle on 2024-09-13 at 06:46

@Aissen @axboe sed -e’s/should/must/g’ and then check if the advice given is sufficient to adhere to the rules.

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Markus Werle on 2024-09-13 at 06:49

@Aissen @axboe replace “it's not uncommon to have a pull request contain multiple commits” by “it's perfectly OK to have a pull request contain multiple commits” to avoid the double negation that let me misread it first and also sounds more positive.

=> More informations about this toot | More toots from markuswerle@nrw.social

Toot

Written by Markus Werle on 2024-09-13 at 06:52

@Aissen @axboe for “squash fixup” please give an example of what you really want to happen. Do you want a git amend for the wrong commit or a local history rewrite pushed? 3 lines of git commands remove all room for interpretation.

=> More informations about this toot | More toots from markuswerle@nrw.social

Descendants

Written by Markus Werle on 2024-09-13 at 06:57

@Aissen @axboe I forgive you the “Don't explain what the code in commit does, that should be readily apparent from just reading the code.“ You are a very intelligent kernel hacker so I fully understand that “just read the code” is an explanation for you. In my world I want a high level explanation of what was intended. But I am a stupid slow learner and cannot grok Linux source code. I only see trees, not the woods.

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Markus Werle on 2024-09-13 at 07:00

@Aissen @axboe do we need “heinous” in

“liburing doesn't squash-on-rebase, or other heinous practices sometimes seen elsewhere.”?

For merge/rebase strategies I would go with YMMV.

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Markus Werle on 2024-09-13 at 07:02

@Aissen @axboe That’s my 2 cents. In general a good thing to have that document.

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Jens Axboe on 2024-09-13 at 13:35

@markuswerle @Aissen No we definitely don't, it's an attempt at humor. But squash or rebase on merge is heinous, objectively, so I'll keep it ;-)

=> More informations about this toot | More toots from axboe@fosstodon.org

Written by Markus Werle on 2024-09-13 at 17:34

@axboe @Aissen ROFL

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Jens Axboe on 2024-09-13 at 13:37

@markuswerle imho that belongs in code comments. The commit message can surely explain what the code does, but I've seen way too many commit messages that just painstakingly detail it. And I don't care about that, I can read it in the code. I care about knowing WHY you felt you need to make that change.

=> More informations about this toot | More toots from axboe@fosstodon.org

Written by Markus Werle on 2024-09-13 at 17:01

@axboe yes, and I fully understand that you do not need that information. For me the lack of that information is the major blocker for understanding the inner parts especially of complicated low level kernel code, BSPs or embedded stuff. The architecture is not available in the meta information, but has to be derived from the code itself which I am not good at.

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Jens Axboe on 2024-09-13 at 17:05

@markuswerle Maybe I wasn't fully clear - what I meant is that if the code does not stand on its own in terms of being understandable or needs explanations, then that should go IN THE CODE, not in the commit message. The commit message should just explain WHY the change is made, not details of how the code works. Generally speaking, obviously there are exceptions, arch type overviews I think would be fine (but still better in the code where people will see them!).

=> More informations about this toot | More toots from axboe@fosstodon.org

Written by Markus Werle on 2024-09-13 at 17:29

@axboe so you mean in the code as comment? That would be ok for me

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Jens Axboe on 2024-09-13 at 17:30

@markuswerle yep exactly

=> More informations about this toot | More toots from axboe@fosstodon.org

Written by Markus Werle on 2024-09-13 at 17:34

@axboe the we are on the same page and I fully agree

=> More informations about this toot | More toots from markuswerle@nrw.social

Written by Jens Axboe on 2024-09-13 at 13:38

@markuswerle good point! What I mean is fixup the original commit, which is amending it with the fixup. I do see people messing up that part, so adding some details on how to do that would be useful. I'll do that.

=> More informations about this toot | More toots from axboe@fosstodon.org