In D146041#4195992, @DavidSpickett wrote:

Since this is my first commit to such a large repository(and project), can you please guide me with this @DavidSpickett !

Sure, you'll want to make a commit that only has changes to warnings and errors. You can split up this one to do that, see part "A)" of this answer https://stackoverflow.com/questions/6217156/break-a-previous-commit-into-multiple-commits/6217314#6217314. Then you can update this review with that new commit, change the description/title etc. if needed.

If you get confused with updating the review (happens to me all the time) you can just abandon this (there is an entry in the "Add Action..." menu) and make a new review as you did before.

How to identify what changes should be included? I would ignore comments, shell scripts, FIXMEs, or general test data. If the test is producing a warning and looking for it, clearly it should be changed. If it's just random data it's using to test some function, I wouldn't change it.

In D146041#4193172, @junaire wrote:

I'm not certain if it's bad to say 'sorry', IMHO it's fine?

Anyway, you can't just simply delete those words (in diagnostic messages and regression tests), that doesn't work. To verify the patch is good, you can run ninja check-all to make sure the testsuite passes.

In D146041#4193045, @DavidSpickett wrote:

Is there some standard for writing warning messages? For llvm that is, it would be worth looking through the getting started guides to see. I think the majority of warnings are "formal" in that sense so this seems fine.

Personally I agree with making the warnings more succinct but aside from that I don't see the need to change comments or testing scripts.

You may consider splitting this change into 2. One that only changes warnings and errors (a less controversial change) and the rest (that is up to the reviewers of each bit).

In D146041#4192816, @mehdi_amini wrote:

You should look into the title and description of the commit: https://cbea.ms/git-commit/