[Starlingx-discuss] Proposed rules regarding patch files
Hi Folks, I'm tired of tripping over the same nonsense again and again regarding patch files in our source repos. Can we consider getting this captured in our StarlingX developer how-to documentation/training materials? Also the build-pkgs tool should have an alternate "--edit" option that applies patches via "git am", making the actual source it produces in a git repo useful, instead of the mangled commit headers it creates currently. -Jim Patch files should always be generated via git format-patch and contain a meaningful commit header, thus being "git am friendly". Patches are not to be generated via quick and dirty diff commands. The commit header should also contain your sign off if you've done more than just a context refresh or simple rebase of the patch. Patch files should never be directly edited. The temptation may be there for you to be lazy and just edit a patch file directly. Do not do this, work in git to correct the patch and then regenerate it. The name of a patch file is generated automatically by git format-patch from the shortlog ie. first line of the commit header. The shortlog is very important in the grand scheme of the commit header, put a lot of thought into writing that one line to make it *meaningful*. Many developers use "git log --oneline" to display lists of commits, and that displays the shortlog only. Again, make it succinct and meaningful, I cannot emphasize this enough. Patch files should never be renamed eg. via the mv command, after they are generated. If you don't like the name of the generated patch file, change the commit shortlog and regenerate it. Do NOT do this crime (example): git format-patch <arguments> produces fix-some-problem-description.patch mv fix-some-problem-description.patch some-description-i-prefer.patch
On Tue, Nov 26, 2019 at 12:49 PM Jim Somerville <jim.somerville@windriver.com> wrote:
Hi Folks,
I'm tired of tripping over the same nonsense again and again regarding patch files in our source repos. Can we consider getting this captured in our StarlingX developer how-to documentation/training materials?
Also the build-pkgs tool should have an alternate "--edit" option that applies patches via "git am", making the actual source it produces in a git repo useful, instead of the mangled commit headers it creates currently.
-Jim
Patch files should always be generated via git format-patch and contain a meaningful commit header, thus being "git am friendly". Patches are not to be generated via quick and dirty diff commands. The commit header should also contain your sign off if you've done more than just a context refresh or simple rebase of the patch.
Patch files should never be directly edited. The temptation may be there for you to be lazy and just edit a patch file directly. Do not do this, work in git to correct the patch and then regenerate it.
The name of a patch file is generated automatically by git format-patch from the shortlog ie. first line of the commit header. The shortlog is very important in the grand scheme of the commit header, put a lot of thought into writing that one line to make it *meaningful*. Many developers use "git log --oneline" to display lists of commits, and that displays the shortlog only. Again, make it succinct and meaningful, I cannot emphasize this enough.
+1 In addition to this I would like to share a presentation from last week kubecon: https://speakerdeck.com/thockin/how-to-be-a-bad-ass-code-reviewer The commit-msg should answer 2 questions: what /why: What are the changes listed on the patch why you want to do the changes Other good points are highlighted in the presentation Hope it helps to improve the quality of our commit msgs Regards Victor R
Patch files should never be renamed eg. via the mv command, after they are generated. If you don't like the name of the generated patch file, change the commit shortlog and regenerate it.
Do NOT do this crime (example):
git format-patch <arguments> produces fix-some-problem-description.patch mv fix-some-problem-description.patch some-description-i-prefer.patch
_______________________________________________ Starlingx-discuss mailing list Starlingx-discuss@lists.starlingx.io http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss
participants (2)
-
Jim Somerville
-
Victor Rodriguez