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

add git commit hash in cargo-contract version #189

Merged
merged 5 commits into from
Apr 1, 2021

Conversation

atenjin
Copy link
Contributor

@atenjin atenjin commented Feb 20, 2021

No description provided.

Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

@atenjin The commit hash is missing in the second command here:

$ cargo run -- --version
cargo-contract 0.8.0-bd0db50-x86_64-linux-gnu
$ cargo run -- contract --version
cargo-contract 0.8.0

Besides that I think it's fine the way you copied the generate_cargo_keys stuff, avoiding polluting the codebase with substrate-named environment variables. So you can remove the comments regarding that.

build.rs Outdated
};

use anyhow::Result;
use platforms::*;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would prefer not having a wildcard import here.

Suggested change
use platforms::*;
use platforms::{TARGET_ARCH, TARGET_ENV, TARGET_OS};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have fixed

@atenjin
Copy link
Contributor Author

atenjin commented Apr 1, 2021

@atenjin The commit hash is missing in the second command here:

$ cargo run -- --version
cargo-contract 0.8.0-bd0db50-x86_64-linux-gnu
$ cargo run -- contract --version
cargo-contract 0.8.0

Besides that I think it's fine the way you copied the generate_cargo_keys stuff, avoiding polluting the codebase with substrate-named environment variables. So you can remove the comments regarding that.

thanks, I find out the place to add version for this tip.

Now I have removed the todo command and added suitable version hint.

Copy link
Collaborator

@cmichi cmichi left a comment

Choose a reason for hiding this comment

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

Thanks @atenjin !

@cmichi cmichi merged commit cacf2d5 into use-ink:master Apr 1, 2021
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