Skip to content

Conversation

@dsully
Copy link
Contributor

@dsully dsully commented May 8, 2021

This will prevent changes when these settings are defined in a different, but compatible way to what volta does.

Copy link
Contributor

@charlespierce charlespierce left a comment

Choose a reason for hiding this comment

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

Thanks @dsully! Looks mostly good, just a couple of small tweaks to make sure things work as expected and are easier to debug later on.

Also, would you mind creating a follow-on issue to add a --force or similar flag? It's not necessary for this PR, but would be good to track the need for something like that in case we want to have the ability to force it through even given the environment appears to be properly set up.

@charlespierce
Copy link
Contributor

Looks like the changes need to have cargo fmt run on them. Also the Windows builds are failing for some unrelated reason, it appears, I'll investigate that separately.

dsully and others added 2 commits May 10, 2021 13:01
Co-authored-by: Charles Pierce <charlespierce@users.noreply.github.com>
@dsully
Copy link
Contributor Author

dsully commented May 10, 2021

Done & done!

Copy link
Contributor

@charlespierce charlespierce left a comment

Choose a reason for hiding this comment

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

One more small change (that may need cargo fmt run again afterwards).

Copy link
Contributor

@charlespierce charlespierce left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! The CI failures are unrelated and fixed in #993, once that's merged I'll merge this and then confirm that everything passes post-merge.

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.

2 participants