Page MenuHomePhabricator

[docs] Update path to clang-tools-extra
ClosedPublic

Authored by AlexanderLanin on Sun, Dec 29, 5:20 PM.

Details

Summary

tools/clang/tools/extra

has become

clang-tools-extra

which was not updated in all docs.

Diff Detail

Event Timeline

AlexanderLanin created this revision.Sun, Dec 29, 5:20 PM
Jim added a subscriber: Jim.Sun, Dec 29, 11:08 PM

There shows "Context not available".
Could you update for this?

Hi,
I don't see the error message you mentioned anywhere, but I'm gonna assume it involves the number of surrounding lines. Here is another attempt with

git show HEAD -U999999 > mypatch.patch


Source of problem:
https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface tells me to use

git format-patch -U999999 @ {u}

But then upload fails:

Diff Parse Exception: Expected a hunk header, like 'Index: /path/to/file.ext' (svn), 'Property changes on: /path/to/file.ext' (svn properties), 'commit 59bcc3ad6775562f845953cf01624225' (git show), 'diff --git' (git diff), '--- filename' (unified diff), or 'diff -r' (hg diff or patch).

        1885        for more information.</p>
        1886    
        1887    </div>
        1888    </body>
        1889    </html>
>>>     1890   -- 
        1891   2.24.1.windows.2
        1892

https://llvm.org/docs/GettingStarted.html#sending-patches just casually mentions git show and git format-patch without telling anything about options to be used.

Assuming 'git show HEAD -U999999' is the one and only correct way? 'git diff HEAD~1 -U999999' seems to work ok also.
However 'git format-patch' doesn't work as mentioned above. At least for me with git version 2.24.1.windows.2

Jim added inline comments.Mon, Dec 30, 6:19 PM
clang-tools-extra/docs/clang-tidy/Contributing.rst
66

I am no sure that "llvm/clang-tools-extra" should be replaced as "llvm-project/clang-tools-extra".
Maybe someone would confuse with "llvm", "llvm-project" and "llvm-project/llvm"

clang/www/hacking.html
301 ↗(On Diff #235577)

This change should be in a separate patch.

aaron.ballman added inline comments.Tue, Dec 31, 6:16 AM
clang-tools-extra/docs/clang-tidy/Contributing.rst
66

Elsewhere we use path/to/llvm/source, which seems to be sufficiently clear.

78

path/to/llvm/source here as well.

AlexanderLanin marked 3 inline comments as done.Wed, Jan 1, 10:59 AM
AlexanderLanin added inline comments.
clang-tools-extra/docs/clang-tidy/Contributing.rst
66

While this goes slightly beyond the scope of the original pull request I tend to agree as llvm can easily be confused with llvm-project/llvm as Jim wrote.
However I'm not clear on the exact target: looking through other docs probably most often you'll find path/to/llvm/source as Aaron mentioned, but other times it's /path/to/llvm-project/, llvm-project/, ~/llvm/, ~/clang-llvm/, /path/to/llvm, /path/to/llvm/src, /path/to/llvm/sources or /path/to/llvm/tree.

While this is not that important, it's difficult enough to get started with anything inside llvm as it is. This is low hanging fruit. I would create a separate pull request afterwards to align those.

clang/www/hacking.html
301 ↗(On Diff #235577)
aaron.ballman accepted this revision.Wed, Jan 1, 12:45 PM

As you mention, we're already inconsistent with how we designate where the repo lives on disk, so I'm fine with landing this as-is and making the root part of the path consistent in a follow-up. LGTM, with the "getting started" changes pulled out.

clang-tools-extra/docs/clang-tidy/Contributing.rst
66

A separate pull request to make them consistent would be a great follow-up to this one.

clang/www/hacking.html
301 ↗(On Diff #235577)

Please be sure to back this change out before landing this patch.

This revision is now accepted and ready to land.Wed, Jan 1, 12:45 PM
AlexanderLanin marked an inline comment as done.
AlexanderLanin edited the summary of this revision. (Show Details)

Removed change in hacking page as discussed.
Can someone commit this as apparently I cannot do it myself (I'll create a PR with updated getting started documentation later...), thanks.

Jim added a comment.Wed, Jan 1, 5:41 PM

@AlexanderLanin
git commit --amend --author="What-is-your-name <What-is-your-email-address>" ?

@Jim git commit --amend --author="Alexander Lanin <alex@lanin.de>"

This revision was automatically updated to reflect the committed changes.