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

Include git hash and crate version #977

Merged
merged 8 commits into from
Feb 6, 2021
Merged

Include git hash and crate version #977

merged 8 commits into from
Feb 6, 2021

Conversation

RajarupanSampanthan
Copy link
Contributor

Summary of changes
Changes introduced in this pull request:

  • Uses git-version crate to get git commit
  • Uses rust env! to get package version

Reference issue to close (if applicable)

Closes
#934

Other information and links

@RajarupanSampanthan RajarupanSampanthan changed the title First commit Include git hash and crate version Feb 2, 2021
// TODO update to include actual version
// https://github.com/ChainSafe/forest/issues/934
format!("forest-{}", "0.1.0"),
format!("forest-{}", *VERSION),
Copy link
Contributor

Choose a reason for hiding this comment

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

nice! Can you add that same commit hash at the end of this as well? Something like

Suggested change
format!("forest-{}", *VERSION),
format!("forest-{}-{}", *VERSION, *CURRENT_COMMIT),

Sorry if the issue was unclear. If not I can just do it in a following PR.

@@ -7,6 +7,7 @@ edition = "2018"
[dependencies]
utils = { path = "../utils" }
networks = { path = "../../types/networks" }
fil_types = { path = "../../types" }
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry for the nit, but can you avoid bringing this dependency into the libp2p crate? Since the type is as simple as just git_version!(), maybe just have that in place of CURRENT_COMMIT

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @nannick, this PR looks great and I'd like to pull it in. The branch just needs to be updated and that last change (#977 (comment)) made. If you give me privileges to modify your branch I can do this for you, if you'd like :)

Sorry for the late fix on such a small PR

@@ -7,6 +7,7 @@ edition = "2018"
[dependencies]
utils = { path = "../utils" }
networks = { path = "../../types/networks" }
fil_types = { path = "../../types" }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fil_types = { path = "../../types" }

@austinabell
Copy link
Contributor

Hey @nannick, this PR looks great and I'd like to pull it in. The branch just needs to be updated and that last change (#977 (comment)) made. If you give me privileges to modify your branch I can do this for you, if you'd like :)

@austinabell
Copy link
Contributor

Thank you so much for this! This will be nice to have

@austinabell austinabell merged commit bd54611 into ChainSafe:main Feb 6, 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.

None yet

2 participants