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.

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

docs(build): add text build steps for Mac #3421

Merged

Conversation

ashnashahgrover
Copy link
Contributor

@ashnashahgrover ashnashahgrover commented Jul 18, 2024

Commit to be reviewed


docs(build): add text build steps for Mac

Primary Changes
----------------
1. Updated BUILD.md to include Mac OS specific installation instructions. 
2. Added placeholders for Linux and Windows specific instructions.

Fixes #1963

Signed-off-by: ashnashahgrover as19@williams.edu

Pull Request Requirements

  • Rebased onto upstream/main branch and squashed into single commit to help maintainers review it more efficient and to avoid spaghetti git commit graphs that obfuscate which commit did exactly what change, when and, why.
  • Have git sign off at the end of commit message to avoid being marked red. You can add -s flag when using git commit command. You may refer to this link for more information.
  • Follow the Commit Linting specification. You may refer to this link for more information.

Character Limit

  • Pull Request Title and Commit Subject must not exceed 72 characters (including spaces and special characters).
  • Commit Message per line must not exceed 80 characters (including spaces and special characters).

A Must Read for Beginners
For rebasing and squashing, here's a must read guide for beginners.

Copy link
Contributor

@jagpreetsinghsasan jagpreetsinghsasan left a comment

Choose a reason for hiding this comment

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

Need doc formatting and specifics like installing go page should be updated, nvm installation steps, etc

Copy link
Member

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@ashnashahgrover Looking good, just a few nit-picks:

  1. Change the commit message and PR title to reflect the contents a little closer:
    You added instructions for Mac and placeholders for Linux, Windows. Someone reading this commit message would most likely end up thinking that they are about to read the instructions on how to set the project up on Linux and the get greeted by disappointment [1].
  2. Explicitly clarify if you are talking about intel or ARM macs unless you've tested the steps on both CPU architectures. If you did test it both and it works that's awesome and we should make sure that it's explicitly stated as well.

[1]:

### Linux 
* Insert Linux instructions here 

@ashnashahgrover ashnashahgrover changed the title docs(build): add text build steps for Linux, Mac docs(build): add text build steps for Mac Jul 22, 2024
@jagpreetsinghsasan
Copy link
Contributor

LGTM too

@petermetz petermetz dismissed jagpreetsinghsasan’s stale review July 31, 2024 15:57

Jagpreet said LGTM in the meantime, he probably just forgot to mark his change request resolved.

Primary Changes
----------------
1. Updated BUILD.md to include Mac OS specific installation instruction
2. Added placeholders for Linux and Windows specific instructions.
 
Fixes hyperledger#1963

Signed-off-by: ashnashahgrover <as19@williams.edu>
@petermetz petermetz merged commit 7621db7 into hyperledger:main Jul 31, 2024
141 of 144 checks passed
@petermetz petermetz deleted the ashnashahgrover/issue1963 branch July 31, 2024 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs(build): add text build steps for Linux, Mac and Windows
4 participants