[PATCH] D56482: DO NOT SUBMIT. Draft for guidelines on using Phabricator.

MyDeveloperDay via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 9 12:14:39 PST 2019


MyDeveloperDay added a comment.

> ! In D56482#1350874 <https://reviews.llvm.org/D56482#1350874>, @kristina wrote:
> 
> I only added a section, all the warnings are from existing bits below. Not sure if it's a good idea to reformat it as part of the diff as it will make it harder to see the added section. Can do it afterwards but that's what's currently being used to generate documentation.

Sorry I wasn't clear, I'm using a script from D55523: [clang-tidy] Linting .rst documentation <https://reviews.llvm.org/D55523> to validate .rst files, if you read the revision there is some info on the rational. (I simply ran it here on a copy of the file you are editing prior to your revision). You'll see from D55523 <https://reviews.llvm.org/D55523> that I mention that I saw a lot of review comments which amounted to stylistic changes in the .rst files, and yet I see a lot of reviewers time pointing these out, time and time again and its all simple stuff that could be automated. (or at least added to such a how to guide)

it would be good if we could ensure .rst files are checked prior to review (how we do this I don't know) without initially generating 10,000 of minor whitespaces changes.

when I reviewed all .rst files currently checked into LLVM I found 10,000's of simple violations (maybe people aren't worried about it but certainly it comes up a lot in reviews and causes the second or third revisions)

I think another point is that lot of review comments could potentially be caught by clang-tidy, there is sometimes lots of the review conversations  especially with new contributors, about "don't use anonymous namspaces but prefer static functions","clang-format the file", don't overly use auto", don't have "const value types" all stuff that is in the CodingStyle but is easily missed when making or updating a review.

It would be good if we had a mechanism to be able to validate a patch prior to commit, maybe using something like clang-tidy-diff.py in combination with something like the .rst linter to reduce the iterations on a revision, speed up development and reduce the burden  that seems present on a group of dedicated but possible select people.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56482





More information about the llvm-commits mailing list