-
Notifications
You must be signed in to change notification settings - Fork 1k
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 recuperation of the current version from git if available. #731
Conversation
Include the version when auto-reporting an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, this should help improve issue requests 👍
One point for consideration in my comment below.
lib/Configuration.php
Outdated
|
||
if(file_exists($revisionHashFile)) { | ||
|
||
return 'git.' . trim(file_get_contents($revisionHashFile)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe include $VERSION
as well?
return Configuration::$VERSION . ' (' . trim(file_get_contents($revisionHashFile)) . ')';
=> 2018-06-10 (937ea492716b9251211a2a0f73f3afd1ac871505)
.git/refs/heads/master
always returns the latest commit in master
! It wouldn't show branches. Here is an alternative implementation for consideration:
Notice: It uses exec
to extract the latest commit hash in short form and reads the hash file only if that operation fails. This is purely cosmetic.
public static function getVersion() {
$headFile = '.git/HEAD';
$version = Configuration::$VERSION;
exec('git log --pretty="%h" -n1 HEAD', $output, $return);
if(!$return) {
$version .= ' (' . trim($output[0]) . ')';
} elseif(file_exists($headFile)) {
$head = trim(explode(':', file_get_contents($headFile))[1]);
$revisionHashFile = '.git/' . $head;
if(file_exists($revisionHashFile)) {
$version .= ' (' . trim(file_get_contents($revisionHashFile)) . ')';
}
}
return $version;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I decided to read the .git/refs/heads/master
is for simplicity: I know it only shows the master
revision, but if we have a bug report, we only consider it for the version we maitain, that is master. Moreover, a lot of shared hosts do not allow exec, which is why I decided to go for simplicity.
However I believe that we could include the branch name in the version string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including the branch name sounds like a much simpler alternative.
Using exec was just for cosmetic reasons (the full hash is quite long), as you can see it'll fall back to reading files if exec isn't working. But I'm not fixated on making this more complex than need be, so just leave it out 😄
Add the branch name to the version.
Modified so it also includes the branch name. I also changed the hash so it is the same as the short hash (7 first chars of the hash) |
Works on my end 👍 |
…idge#731) * Add recuperation of the current version from git if available * Include version when auto-reporting an error
Include the version when auto-reporting an error.
When creating a new release, we can just update the version in the configuration class.