Skip to content
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

Update BUILD.md #4514

Merged
merged 1 commit into from
May 31, 2023
Merged

Update BUILD.md #4514

merged 1 commit into from
May 31, 2023

Conversation

oeggert
Copy link
Collaborator

@oeggert oeggert commented Apr 28, 2023

High Level Overview of Change

This PR updates the BUILD.md instructions. This mainly fixes formatting and restructures content for better readability and understanding.

Context of Change

This change was prompted by this discussion: #4433

Type of Change

  • [x ] Documentation Updates

@oeggert oeggert mentioned this pull request Apr 28, 2023
Closed
@oeggert oeggert self-assigned this Apr 28, 2023
@oeggert oeggert added the Documentation README changes, code comments, etc. label Apr 28, 2023
HowardHinnant
HowardHinnant approved these changes May 30, 2023
View reviewed changes
Copy link
Contributor

@HowardHinnant HowardHinnant left a comment

Choose a reason for hiding this comment

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

A few recommendations:

  1. When building on Apple M silicon, I believe that the Conan install option --build missing needs to be changed to just --build.

  2. I recommend adding an example to the options section that shows how to use an option. Perhaps:

cmake -DCMAKE_TOOLCHAIN_FILE: ... -Dassert=ON
  1. One can confirm how options are currently set in CMakeCache.txt

@intelliot
Copy link
Collaborator

I will be merging this shortly; let's consider Howard's suggestions above for a future PR.

@intelliot intelliot added this to the 1.11.0 milestone May 31, 2023
@intelliot intelliot merged commit adde0c2 into XRPLF:develop May 31, 2023
@intelliot intelliot mentioned this pull request Jun 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@intelliot intelliot intelliot approved these changes

@HowardHinnant HowardHinnant HowardHinnant approved these changes

@thejohnfreeman thejohnfreeman Awaiting requested review from thejohnfreeman

@mDuo13 mDuo13 Awaiting requested review from mDuo13

Assignees

@oeggert oeggert

Labels
Documentation README changes, code comments, etc. Trivial
Projects
Core Ledger
Archived in project
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.
3 participants
@oeggert @intelliot @HowardHinnant