Page MenuHomePhabricator

[doc][cmake] Convert read-me for the common CMake utils to reST
ClosedPublic

Authored by Ericson2314 on Jan 3 2022, 12:02 AM.

Details

Summary

@phosek mentioned others might want it reST for consistency. As I
personally do not like Markdown at all and just did the "usual GitHub
read-me thing" out of habit, I am more than happy to oblige.

Also fix the typos found in the original.

Diff Detail

Event Timeline

Ericson2314 created this revision.Jan 3 2022, 12:02 AM
Ericson2314 requested review of this revision.Jan 3 2022, 12:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2022, 12:02 AM

Coming from https://reviews.llvm.org/D116477#inline-1114179, just let me know what style you think is best for the tools and runtime libs alike, @idionne, and I will happy go update all of them + the readme to match.

lebedev.ri accepted this revision.Jan 7 2022, 11:46 AM
lebedev.ri added a subscriber: lebedev.ri.

Hmm, i guess this does seem sound.
I'd like for someone else to comment (@beanz ? not sure who, out of those that are still around, owns cmake), though.

This revision is now accepted and ready to land.Jan 7 2022, 11:46 AM
Ericson2314 retitled this revision from [cmake] Add read me for the common CMake utils to [cmake] Add read-me for the common CMake utils.Jan 7 2022, 11:54 AM
Ericson2314 edited the summary of this revision. (Show Details)
This revision was landed with ongoing or failed builds.Jan 7 2022, 12:14 PM
This revision was automatically updated to reflect the committed changes.
sterni added a subscriber: sterni.Jan 7 2022, 12:17 PM
sterni added inline comments.
cmake/README.md
9

typo: should be *uses* them

phosek reopened this revision.Jan 7 2022, 12:45 PM

This looks reasonable, thank you for writing it. The only thing I'm not sure about is the format. We use reStructuredText for most documentation in LLVM although we do have a handful of Markdown documents in the tree. Personally, I'm fine with either format but others may have stronger opinion on this.

cmake/README.md
44

I'd say "For runtime libraries" instead.

This revision is now accepted and ready to land.Jan 7 2022, 12:45 PM
beanz added a comment.Jan 7 2022, 12:48 PM

LGTM too. I think markdown is probably better than rst for documentation files that aren't part of the sphinx doc builds, so I'd leave this as md.

I will fix the typo and convert it. I actually agree Markdown is terrible on principle, but just was doing the "usual thing for readmes on github". Even though it is not being fed to sphinx git can still be in reST.

I will make a new commit reverting it.

And sorry I already landed it. Was discussing on IRC and thought the only thing that happened was it getting a second approval from @beanz.

Convert to reST

Ericson2314 retitled this revision from [cmake] Add read-me for the common CMake utils to [cmake] Convert read-me for the common CMake utils to reST.Jan 7 2022, 10:13 PM
Ericson2314 edited the summary of this revision. (Show Details)

I'll let this sit a bit in case anyone wants to re-review it.

sterni added a comment.Jan 8 2022, 1:23 AM

I'll let this sit a bit in case anyone wants to re-review it.

The typos / wording stuff pointed out above still apply, I think.

Ericson2314 retitled this revision from [cmake] Convert read-me for the common CMake utils to reST to [doc][cmake] Convert read-me for the common CMake utils to reST.Jan 8 2022, 11:56 AM
Ericson2314 edited the summary of this revision. (Show Details)
Ericson2314 added inline comments.
cmake/README.rst
13

Fixed typo

51

Fixed typo

phosek accepted this revision.Jan 10 2022, 1:33 AM