Skip to content

doc: update documentation and scripts related to build directories #30741

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Sep 3, 2024
Merged

doc: update documentation and scripts related to build directories #30741

merged 2 commits into from
Sep 3, 2024

Conversation

l0rinc
Copy link
Contributor

@l0rinc l0rinc commented Aug 28, 2024 •
edited
Loading

In the other readmes we've provided a default build directory instead, unified the developer-notes.md to specify it explicitly.

In the next commit I've used this default to go over each reference to our binaries and changed their in-source references to the build directory.
Some of these changes were in example outputs - I haven't validated that the outputs are still the same.
I haven't modified the build folders in the devtools.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 28, 2024 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK pablomartin4btc, maflcko, fanquake
Concept ACK theStack, 0xB10C
Stale ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29877 (tracing: explicitly cast block_connected duration to nanoseconds by 0xB10C)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

fanquake
fanquake reviewed Aug 28, 2024
View reviewed changes
@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from 00c12c6 to 84de9e3 Compare August 28, 2024 15:11
fanquake
fanquake reviewed Aug 28, 2024
View reviewed changes
@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from 84de9e3 to d21f59d Compare August 28, 2024 15:14
@l0rinc l0rinc changed the title Update documentation generation example in developer-notes.md cmake: update documentation generation example in developer-notes.md Aug 28, 2024
@l0rinc l0rinc changed the title cmake: update documentation generation example in developer-notes.md cmake: update documentation and scripts related to build directories Aug 28, 2024
@l0rinc l0rinc changed the title cmake: update documentation and scripts related to build directories doc: update documentation and scripts related to build directories Aug 28, 2024
fanquake
fanquake reviewed Aug 28, 2024
View reviewed changes
@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from d21f59d to 3a4a143 Compare August 29, 2024 06:44
@TheCharlatan TheCharlatan mentioned this pull request Aug 29, 2024
Merged
TheCharlatan
TheCharlatan reviewed Aug 29, 2024
View reviewed changes
@hebasto
Copy link
Member

hebasto commented Aug 29, 2024

Why is this a draft?

@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from 3a4a143 to cd993bf Compare August 29, 2024 10:11
@l0rinc
Copy link
Contributor Author

l0rinc commented Aug 29, 2024

Why is this a draft?

master wasn't stable yet, ready for review!

@l0rinc l0rinc marked this pull request as ready for review August 29, 2024 10:12
@DrahtBot DrahtBot mentioned this pull request Aug 29, 2024
Merged
@hebasto hebasto added Docs and removed Build system labels Aug 29, 2024
hebasto
hebasto approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@hebasto hebasto left a comment •
edited
Loading

Choose a reason for hiding this comment

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

ACK cd993bf, I have reviewed changes and they look OK.

The failure in the "test each commit" CI job is unrelated.

fanquake
fanquake reviewed Aug 29, 2024
View reviewed changes
@DrahtBot DrahtBot mentioned this pull request Aug 29, 2024
Merged
maflcko
maflcko reviewed Aug 29, 2024
View reviewed changes
@fanquake
Copy link
Member

fanquake commented Aug 29, 2024 •
edited
Loading

Can you fixup all the commits to have proper prefixes, commit messages etc.

Also, I don't think putting changes into commits based on if you actually tested things on not, make that much sense. I guess that's for your own convenience so it's easier for you to can drop the changes if they happen to be broken? However it doesn't really benefit contributors, or the changelog.

@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from cd993bf to 4c5e58c Compare August 29, 2024 13:22
l0rinc and others added 2 commits August 29, 2024 15:22
@l0rinc @pablomartin4btc
doc: Update documentation generation example in developer-notes.md
91b3bc2 
To correspond to the documentation style of e.g. src/test/README.md

Co-authored-by: pablomartin4btc <pablomartin4btc@gmail.com>
@l0rinc
doc: Prepend 'build/' to binary paths under 'src/' in docs
6a68343
@l0rinc l0rinc force-pushed the lorinc/build-documentation branch from 4c5e58c to 6a68343 Compare August 29, 2024 13:23
@l0rinc
Copy link
Contributor Author

l0rinc commented Aug 29, 2024 •
edited
Loading

commits based on if you actually tested things or not

That's also why it was a draft at first, so that I can get feedback on how to proceed with the devtools files.
I've reverted them, it's ready for review now, thanks for the feedback.

pablomartin4btc
pablomartin4btc approved these changes Aug 29, 2024
View reviewed changes
Copy link
Member

@pablomartin4btc pablomartin4btc left a comment

Choose a reason for hiding this comment

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

ACK 6a68343

@DrahtBot DrahtBot requested a review from hebasto August 29, 2024 19:19
@DrahtBot DrahtBot mentioned this pull request Aug 31, 2024
Merged
@fanquake fanquake mentioned this pull request Sep 2, 2024
Closed
@theStack
Copy link
Contributor

theStack commented Sep 2, 2024

Concept ACK

1 similar comment
@0xB10C
Copy link
Contributor

0xB10C commented Sep 2, 2024

Concept ACK

@maflcko
Copy link
Member

maflcko commented Sep 3, 2024

review ACK 6a68343

@DrahtBot DrahtBot requested review from 0xB10C and theStack September 3, 2024 06:18
fanquake
fanquake approved these changes Sep 3, 2024
View reviewed changes
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 6a68343 - we still need to followup with other scripts/devtools, and likely unify what we are doing in some way, but this is an improvement.

@fanquake fanquake merged commit 4c526f5 into bitcoin:master Sep 3, 2024
16 checks passed
@LarryRuane LarryRuane mentioned this pull request Sep 10, 2024
Merged
@l0rinc l0rinc deleted the lorinc/build-documentation branch September 12, 2024 21:00
@torkelrogstad torkelrogstad mentioned this pull request Sep 28, 2024
Merged
fanquake added a commit that referenced this pull request Oct 15, 2024
@fanquake
Merge #30859: doc: cmake: prepend "build" to functional/test_runner.py
15563d3 
e64b2f1 doc: cmake: prepend and explain "build/" where needed (Larry Ruane)

Pull request description:

  This is a small follow-on to #30741, prepend `build/` to the path for `test_runner.py`.

ACKs for top commit:
  jonatack:
    ACK e64b2f1
  maflcko:
    lgtm ACK e64b2f1
  tdb3:
    re ACK e64b2f1

Tree-SHA512: 80943d4f342987bf060adacb1c7db2e9ff8de5a6da592846ba23f230281d3a5b306162c4c86e61739a29323eaa4abf09f69f41302996d5809f448e5788a74a87
fanquake added a commit that referenced this pull request Oct 16, 2024
@fanquake
Merge #30996: doc: update signet documentation related to build direc…
479715e 
…tories

a647d44 doc: update signet documentation related to build directories (Torkel Rogstad)

Pull request description:

  While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in #30741

ACKs for top commit:
  maflcko:
    lgtm ACK a647d44
  pablomartin4btc:
    ACK a647d44
  tdb3:
    Code review and light test ACK a647d44

Tree-SHA512: ac7c3806e0ff65860c41d7b7bdad538368d8a6d8d289c10f9714804f963bafd3a9658301b6697f110f5462a92826b62770963508d5eebf88bf9a0a8442d9f72d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fanquake fanquake fanquake approved these changes

@hebasto hebasto Awaiting requested review from hebasto

@0xB10C 0xB10C Awaiting requested review from 0xB10C

@theStack theStack Awaiting requested review from theStack

+3 more reviewers

@maflcko maflcko maflcko left review comments

@TheCharlatan TheCharlatan TheCharlatan left review comments

@pablomartin4btc pablomartin4btc pablomartin4btc approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Docs
Projects
No open projects
CMake
Status: CMake follow-ups
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@l0rinc @DrahtBot @hebasto @fanquake @theStack @0xB10C @maflcko @TheCharlatan @pablomartin4btc