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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/docs/MyFirstTypoFix.rst:324
+   $ git checkout -b myfirstpatch
+   $ git commit -am "Clarify -Winfinite-recursion message"
+
----------------
mehdi_amini wrote:
> 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*))
> 
> 
Sorry, to be clear: I disagree that documenting the convention of using tags is inappropriate for this documentation at this time.

Tags have been used in commit descriptions from before the commit message was used in email titles (previously the title was a list of the files touched - so the title gave some sense of what part of the project the patch related to - albeit rather roughly/poorly) - but they're even more useful since that change (~8 years ago), eg: https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20130128/163757.html

But sure, if you fundamentally object to it being done in this patch, so be it. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100714