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

Tue Apr 27 09:16:41 PDT 2021

kuhnel added inline comments.

================
Comment at: llvm/docs/MyFirstTypoFix.rst:324
+   $ git checkout -b myfirstpatch
+   $ git commit -am "Clarify -Winfinite-recursion message"
+
----------------
kuhnel wrote:
> dblaikie wrote:
> > mehdi_amini wrote:
> > > dblaikie wrote:
> > > > 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. 
> > > Yeah I fundamentally object to a tutorial introducing recommendation for this while there is no documentation and guidelines for it.
> > > If you want to propose a documentation for a guideline regarding this, why not, but please do as a separate patch and make sure it is documented outside of a tutorial.
> > My perspective is that this would be a fine place to include such a suggestion (I don't think as an explicit statement of what you must do, but as a suggestion of some things people do if you think that might clarify things, etc) - in the same way I would in mailing list usage (& have, several times - asking people to flag NFC changes as such as it makes it easier to review). I don't think everything in this document has to be (& I doubt it is, though I haven't read the whole thing) only things that are already formalized in other documentation. (for instance the heuristics mentioned under "Commit access" don't seem to be documented elsewhere - but I don't object to them either, I think it's reasonable to describe to newcomers some norms about how people generally interact in the community))
> Does someone have a (complete) list of tags to be used and their semantics?
Just looked at the commit history: https://github.com/llvm/llvm-project/commits/main

I can't really see a consistent set of tags people are using. Sometimes it's sub-prrojects (e.g. mlir, clangd) or architectures (ARM, x86) or features or change semantic (NFC). So I guess it would be hard to explain how to pick the right tag.

IMHO the only thing we can say here is that they exist and contributors should try to guess something reasonable...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100714