Skip to content

cmake: Freshen up docs/INSTALL.cmake#12772

Closed
levitte wants to merge 1 commit intocurl:masterfrom
levitte:freshen-INSTALL.cmake
Closed

cmake: Freshen up docs/INSTALL.cmake#12772
levitte wants to merge 1 commit intocurl:masterfrom
levitte:freshen-INSTALL.cmake

Conversation

@levitte
Copy link
Copy Markdown
Contributor

@levitte levitte commented Jan 24, 2024 •
edited
Loading

  • Turn docs/INSTALL.cmake into a proper markdown file, docs/INSTALL-CMAKE.md
  • Move things around to divide the description into configuration, building
    and installing sections
  • Mention the more modern cmake options to configure, build and install, but
    also retain the older variants as fallbacks

vszakats
vszakats reviewed Jan 24, 2024
View reviewed changes
Comment thread docs/INSTALL.cmake Outdated
@levitte levitte force-pushed the freshen-INSTALL.cmake branch from d4b0c5a to f9752fb Compare January 24, 2024 13:08
@levitte levitte marked this pull request as draft January 24, 2024 13:36
bagder
bagder reviewed Jan 26, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@bagder bagder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe change this document to use a .md extension so that it will be treated as markdown properly by GitHub, editors etc?

@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 26, 2024

Maybe change this document to use a .md extension so that it will be treated as markdown properly by GitHub, editors etc?

Sure. INSTALL.cmake.md, then?

@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 26, 2024

Fixup commit added, for separate review. I'll squash it into the first commit later

@levitte levitte force-pushed the freshen-INSTALL.cmake branch 4 times, most recently from 81c003c to d57a721 Compare January 27, 2024 08:55
@github-actions github-actions bot added the CI Continuous Integration label Jan 27, 2024
@levitte levitte force-pushed the freshen-INSTALL.cmake branch from d57a721 to 19793fb Compare January 27, 2024 09:36
@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 27, 2024

I listened to both criticism (the minimum cmake requirement) and ideas (turn it into a proper markdown file) and took a stab at trying to answer them all. I hope that what I've now pushed does the trick.

@levitte levitte force-pushed the freshen-INSTALL.cmake branch 2 times, most recently from 6bd0d5c to c020003 Compare January 27, 2024 09:42
@levitte levitte marked this pull request as ready for review January 27, 2024 09:43
@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 27, 2024

... with that, I can't see this PR being a draft any more.

@levitte levitte force-pushed the freshen-INSTALL.cmake branch 2 times, most recently from 9dc8357 to 482b851 Compare January 27, 2024 09:46
@bagder bagder requested a review from vszakats January 27, 2024 11:45
@vszakats
Copy link
Copy Markdown
Member

vszakats commented Jan 27, 2024

I'd suggest naming the file INSTALL-CMAKE.md. This matches what we use in that directory and avoids the double-extension.

@levitte levitte force-pushed the freshen-INSTALL.cmake branch from 482b851 to 6e64f77 Compare January 27, 2024 21:52
@levitte
Copy link
Copy Markdown
Contributor Author

levitte commented Jan 27, 2024

I'd suggest naming the file INSTALL-CMAKE.md. This matches what we use in that directory and avoids the double-extension.

Wish granted!

@levitte
cmake: Freshen up docs/INSTALL.cmake
a7f8357 
- Turn docs/INSTALL.cmake into a proper markdown file, docs/INSTALL.cmake.md
- Move things around to divide the description into configuration, building
  and installing sections
- Mention the more modern cmake options to configure, build and install, but
  also retain the older variants as fallbacks
@levitte levitte force-pushed the freshen-INSTALL.cmake branch from 6e64f77 to a7f8357 Compare January 27, 2024 21:53
@vszakats
Copy link
Copy Markdown
Member

vszakats commented Jan 27, 2024

Thank you @levitte, LGTM!

@bagder bagder closed this in 0f4c19b Jan 27, 2024
@bagder
Copy link
Copy Markdown
Member

bagder commented Jan 27, 2024

Thanks!

@levitte levitte deleted the freshen-INSTALL.cmake branch February 13, 2024 12:01
vszakats added a commit to vszakats/curl that referenced this pull request Jan 16, 2025
@vszakats
GHA/labeler: fix INSTALL-CMAKE.md reference
57a31b9 
Follow-up to 0f4c19b curl#12772
@vszakats vszakats mentioned this pull request Jan 16, 2025
Closed
vszakats added a commit that referenced this pull request Jan 16, 2025
@vszakats
GHA/labeler: fix INSTALL-CMAKE.md references
f241ccf 
Follow-up to 0f4c19b #12772

Closes #16021
vszakats added a commit that referenced this pull request Jan 26, 2025
@vszakats
projects: fix INSTALL-CMAKE.md references
7c198b5 
Follow-up to 0f4c19b #12772
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
@vszakats @pps83
GHA/labeler: fix INSTALL-CMAKE.md references
8cc0826 
Follow-up to 0f4c19b curl#12772

Closes curl#16021
pps83 pushed a commit to pps83/curl that referenced this pull request Apr 26, 2025
@vszakats @pps83
projects: fix INSTALL-CMAKE.md references
ba33453 
Follow-up to 0f4c19b curl#12772
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bagder bagder bagder left review comments

@vszakats vszakats vszakats approved these changes

Assignees

No one assigned

Labels

build CI Continuous Integration cmake documentation

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@levitte @vszakats @bagder