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

adds rustfmt instructions to supported dev env #6570

Merged
merged 4 commits into from
Jun 10, 2019
Merged

Conversation

nellshamrell
Copy link
Contributor

Signed-off-by: Nell Shamrell [email protected]

@chef-expeditor
Copy link
Contributor

Hello nellshamrell! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

@nellshamrell
Copy link
Contributor Author

@chefsalim and @baumanj - thanks for the feedback. I have combined your suggestions and updated the doc!

```
rustup toolchain install nightly nightly-2019-03-04
rustup component add rustfmt --toolchain nightly-2019-03-04 rustfmt
```
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @baumanj's suggestion (#6570 (comment)) was, rather than hard-code a specific nightly version in this example, directly source the contents of the file that we keep updated with the latest nightly version that works.

That way, you can just run that command, and it will get the version we're currently using.

Copy link
Contributor

Choose a reason for hiding this comment

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

☝️

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think we should suggest people to just run

cargo +"$(< RUSTFMT_VERSION)" fmt

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't that only work at the root level of the Habitat project, though? I tried it from within a component directory and it did not work.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a command you'd want to run from the root. If run from a component directory, I think the

bash: RUSTFMT_VERSION: No such file or directory

error should be sufficient to let users know what's gone wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd much rather err on the side of clarity and be explicit.

@nellshamrell
Copy link
Contributor Author

Just updated this based on suggestions!

@nellshamrell nellshamrell merged commit b4c0e3b into master Jun 10, 2019
chef-ci added a commit that referenced this pull request Jun 10, 2019
Obvious fix; these changes are the result of automation not creative thinking.
@christophermaier christophermaier deleted the update-dev-docs branch May 4, 2020 17:55
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.

5 participants