Server version publication improvements #2838
Conversation
| if ( isset($server_version) && isset($git_commit) ) { | ||
| echo "Server version: $server_version (<a href=https://github.com/BOINC/boinc/commit/$git_commit>$git_commit</a>)<br>"; | ||
| global $git_url; | ||
| if ( isset($server_version) && isset($git_url) ) { |
There was a problem hiding this comment.
I recommend to make the link optional but always show the version (if available). Right now $git_url is set in a way that makes the following assumptions:
- the source repo was cloned via
https, notssh - the source repo's relevant remote is called "origin"
I think that both assumptions shouldn't be made. At least the code should prepare for the case that they're no met, i.e. make the link/URL optional and don't just assume you got a "working" $git_url.
There was a problem hiding this comment.
The release.inc file can always be edited/deleted.
There was a problem hiding this comment.
What's the point of this PR if projects are meant to edit release.inc manually afterwards? What I'm trying to convey here is that the current implementation makes assumptions that won't always be met. Even worse, it doesn't handle gracefully the case when the assumptions aren't met, producing an invalid HTML link.
| f.close() | ||
| os.chmod(os.path.join(dest_dir, 'release.inc'), 0o644) | ||
| '''.format(git_url=git_url, version=version) | ||
| f = open(os.path.join(dest_dir, 'release.inc'), 'w') |
There was a problem hiding this comment.
Is this file only written once during the initial project instantiation? Shouldn't the version be a dynamic detail that always represents the current state?
There was a problem hiding this comment.
It is also created when doing upgrade.
There was a problem hiding this comment.
I see. That's ok as a start but I really think this should be dynamic. Since you're not using --abbrev=0 you show the number of commits since the tag and the local SHA1. This means you think these are interesting details. If, however, the version detail only gets updated with a formal server code update via the BOINC update script, you will miss any other change to the deployed codebase. I think this should be generated by a daily project task.
|
It has now been done a different way. |
These improvements were the result of discussions with Einstein@home.