Conversation
| return 1 | ||
| fi | ||
|
|
||
| if [ -z "${VER_NSIS}" ] |
There was a problem hiding this comment.
It's generally best practice to use double brackets [[ ]]
| VER_MAJ=$(echo "$VERSION" | cut -d. -f1) | ||
| VER_MIN=$(echo "$VERSION" | cut -d. -f2) | ||
| VER_REV=$(echo "$VERSION" | cut -d. -f3) | ||
| if [ "" == "$VER_REV" ]; then |
There was a problem hiding this comment.
Single Brackets (Dangerous):
if [ $name == "Bob" ]
If $name is empty, Bash sees: if [ == "Bob" ], which is a syntax error.
Double Brackets (Safe):
if [[ $name == "Bob" ]]
Even if $name is empty, Bash handles it gracefully without requiring extra quotes.
I try to stick to double brackets everywhere for consistency.
| VER_FULL="$VER_MAJ.$VER_MIN.$VER_REV.$VER_GITREV-$VER_DATE-$VER_GITHASH" | ||
| BUILDTIME=$(date "+%Y.%m.%d %H:%M") | ||
|
|
||
| if [ -z "${SEMVER}" ] || [ -z "${PROJECTNAME}" ] |
There was a problem hiding this comment.
I would use double brackets and you can just do "$SEMVER" and "$PROJECTNAME"
| else | ||
| # Variables passed in. Make use of them. | ||
|
|
||
| VER_FULL="${SEMVER}" |
|
I added some comments. BTW, do you think it would be better for PRs to fork the Oolite repo, then make PRs from the fork keeping the Oolite repo clean with only master and version branches (and maybe a develop branch)? |
|
Windows Test version fails to start with this error
Haven't tried the standard version. |
Added the double brackets although I am more happy with a syntax error than some automagical fix. Your idea of handing development elsewhere is interesting. I was close to suggesting you to use branches in the oolite repository, thus making your development more visible to others. But I am happy to see now we have prereleases available for download for all branches and pull requests. Which means I can make a change and everybody and download and test the behaviour even without having merged. And for guys like you who develop in private then come up with a pull request, the PR will also result in a prerelease so we can test the behaviour before having to merge. |
3 participants
This second semver PR activates the semantic version number in our builds. With that #616 is complete.