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 commit hash to debug version string #4024

Closed
wants to merge 1 commit into from

Conversation

RichardAH
Copy link
Collaborator

High Level Overview of Change

With so many people experimenting with different builds, it can be hard to track which build corresponds to which specific code change. To this end adding the commit hash to the version string when building using -DCMAKE_BUILD_TYPE=Debug will help reduce confusion and human error.

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

@RichardAH
Copy link
Collaborator Author

We might consider actually adding the git commit hash to the release version string as well, it will assist in debugging the live network

Copy link
Contributor

@nbougalis nbougalis left a comment

Choose a reason for hiding this comment

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

I'm fine with this as is (upto my comment) but it might also be interesting to:

a) look at reworking the 64-bit encoded version, so that it can include the commit ID; and
b) decide whether the commit ID should be included in getVersionString or only in getFullVersionString.

Comment on lines +40 to +42
#ifdef GIT_COMMIT_HASH
"-" GIT_COMMIT_HASH
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really your intention to have this block only added when compiling builds with DEBUG or SANITIZER support? If not, you should probably move this block up to line 38.

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