[PATCH] D100714: Add a new tutorial that talk about how to make a change to llvm.

Mehdi AMINI via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 21 16:19:18 PDT 2021


mehdi_amini added inline comments.


================
Comment at: llvm/docs/MyFirstTypoFix.rst:324
+   $ git checkout -b myfirstpatch
+   $ git commit -am "Clarify -Winfinite-recursion message"
+
----------------
dblaikie wrote:
> mehdi_amini wrote:
> > xgupta wrote:
> > > mehdi_amini wrote:
> > > > mehdi_amini wrote:
> > > > > xgupta wrote:
> > > > > > Beginners often forget to add a tag like for this patch Zhiqian didn't add [Docs] tag :)
> > > > > > Should be mention here.
> > > > > Is there a reference in an existing doc about the use of tags? I'm not sure we should tell beginners about things that aren't our documented norms.
> > > > > (I don't use any tag other than "NFC")
> > > > (I wouldn't see the need to add a tag to this very clear and explicit commit title for example)
> > > So It depends on the person, LLVM Project is large with around 100 commits a day. I don't read the complete line of commit/patch. A tag is useful for me.
> > > 
> > > So if it is not documented, we can document it. Otherwise, I left this on other reviewers and patch author to decide what is best.
> > Regardless, this seems to me to be out-of-line for a tutorial to define new conventions.
> Eh, I disagree a bit here - it's the sort of feedback I'd give someone on the mailing list, so might as well write it down here - can be written down other places too.
You're not clear with what you are disagreeing exactly: there is my point that a tutorial should not introduce new conventions, and whether in this case this is a documented convention or something that we've been consistently doing for a while.

We can talk about the latter, but I disagree that this is a settled thing which goes back to my former point: this isn't the revision to settle this.

(to expand a bit, even if this is out-of-scope for this revision I belive: this practice seems like a recent thing people have been doing, and it hasn't been introduced in a coordinated way and without clear guidelines as far as I know. In practice I see this as *noise* and I disagree that this a good use of the commit title limited space. (common guidelines are for git commit titles to be *short*))




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100714/new/

https://reviews.llvm.org/D100714



More information about the llvm-commits mailing list