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

Document installation with cargo-binstall #2471

Merged
merged 2 commits into from
Oct 30, 2023
Merged

Document installation with cargo-binstall #2471

merged 2 commits into from
Oct 30, 2023

Conversation

ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Oct 30, 2023

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@ilyagr ilyagr added the documentation Improvements or additions to documentation label Oct 30, 2023
@ilyagr ilyagr force-pushed the binstall branch 2 times, most recently from eb46cee to 90265b6 Compare October 30, 2023 05:13
@ilyagr ilyagr marked this pull request as ready for review October 30, 2023 05:13
@ilyagr ilyagr force-pushed the binstall branch 2 times, most recently from faa5758 to 0aceb60 Compare October 30, 2023 05:17
@ilyagr
install-and-setup.md: Document binary installation with cargo-binstall
44f3e1a 
I also mentioned this in the changelog, since people reading it are more likely
to use `binstall` than most.
@ilyagr
Copy link
Contributor Author

ilyagr commented Oct 30, 2023

One alternative I'm considering is we could introduce a "Cross-Platform" section after the "Windows" section and move cargo-binstall there. I think I'm mildly leaning to what I have now, but let me know what you prefer.

martinvonz
martinvonz approved these changes Oct 30, 2023
View reviewed changes
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

I think what you have now seems good. Thanks.

thoughtpolice
thoughtpolice reviewed Oct 30, 2023
View reviewed changes
Copy link
Member

@thoughtpolice thoughtpolice 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.

I think this is OK, I'm personally neutral on supporting cargo binstall; it's mostly a convenience. That said I think the installation page just needs an overhaul. Maybe I'll give that a shot later.

docs/install-and-setup.md Show resolved Hide resolved
@ilyagr ilyagr merged commit 38daa9a into jj-vcs:main Oct 30, 2023
14 checks passed
@ilyagr ilyagr deleted the binstall branch October 30, 2023 22:12
@ilyagr
Copy link
Contributor Author

ilyagr commented Oct 30, 2023

BTW, while I don't use BInstall for jj, I use it constantly for development tools like nextest and insta, as well as tools that happen to be written in rust like broot (which I don't use much, but that's another story) or term-transcript

@thoughtpolice thoughtpolice mentioned this pull request Oct 30, 2023
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thoughtpolice thoughtpolice thoughtpolice left review comments

@martinvonz martinvonz martinvonz approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ilyagr @thoughtpolice @martinvonz