This is an archive of the discontinued LLVM Phabricator instance.

[docs] GettingStarted.rst cmake should reference build
ClosedPublic

Authored by fzakaria on Jun 25 2023, 1:47 PM.

Details

Summary

The next sections in GettingStarted assume you are still in the root
directory llvm-project when using ninja.

Make the cmake --build command match it as well.

Note: I am a new cmake user and this confused me.

Diff Detail

Event Timeline

fzakaria created this revision.Jun 25 2023, 1:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2023, 1:48 PM
fzakaria requested review of this revision.Jun 25 2023, 1:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2023, 1:48 PM
fhahn added a subscriber: fhahn.Jul 3 2023, 10:49 AM
fhahn added inline comments.
llvm/docs/GettingStarted.rst
94–95

Should this also be updated to use --build build?

fzakaria added inline comments.Jul 14 2023, 7:15 PM
llvm/docs/GettingStarted.rst
94–95

Ah -- i'm not sure why i never received email for this.
Sorry I missed your feedback.

I think you are right.

fzakaria updated this revision to Diff 540616.Jul 14 2023, 7:32 PM

Add missing build in documentation from review comment

fzakaria updated this revision to Diff 540618.Jul 14 2023, 7:38 PM
  • added missing build specified in docs

@fhahn thank you for the review and sorry for the wait.

Took me a little bit to remember how to use arcanist (it's been a while).
I was going through the GettingStarted guide and got bit again by this so it reminded me to check the review :)

fzakaria marked an inline comment as done.Jul 14 2023, 7:40 PM
MaskRay accepted this revision.Jul 14 2023, 8:18 PM
MaskRay added a subscriber: MaskRay.

Looks great!

This revision is now accepted and ready to land.Jul 14 2023, 8:18 PM

Looks great!

ty :)

Following the https://llvm.org/docs/Phabricator.html; obligatory mention "I do not have commit acces".

MaskRay retitled this revision from fix: GettingStarted.rst cmake should reference build to [docs] GettingStarted.rst cmake should reference build.Jul 15 2023, 11:32 AM
MaskRay added inline comments.
llvm/docs/GettingStarted.rst
94–95

I will change make to make -C build while here.

BTW, you can build the docs with ninja docs-llvm-html. It requires -DLLVM_ENABLE_SPHINX=ON and pip3 install --user recommonmark sphinx sphinx-automodapi (newer pip may need --break-system-packages, I haven't figured it out yet)

This revision was automatically updated to reflect the committed changes.