Closed bryce-carson closed 9 months ago
I take it from the unchecked checklist that I should not merge this yet. I'll proceed towards tagging the release, since it looks you're waiting on me for that now. :-> Thanks!
I take it from the unchecked checklist that I should not merge this yet. I'll proceed towards tagging the release, since it looks you're waiting on me for that now. :-> Thanks!
I had thought to merge it right away, but it does makes more sense to merge after the release. Once it is tagged, I'll work as quick as I can to get the AppImage uploaded and then edit the README again so that people know what's going on when they get here from reading the manual.
Hi @bryce-carson. The 4.1 release is now tagged; have at it! :-> Let me know when I should merge this PR. Thanks!
@bhaller Hi Ben,
I noticed that you haven't created a release for 4.1 yet; would you like to include the AppImage of SLiMgui on MesserLab/SLiM in the release, or should I release it on my fork (since it is a community binary) and it can be linked to from this repository as we've done in the past with other binary releases?
@bhaller I'm not able to get the Git SHA-1 for some reason. I made a fresh clone of the repository, checked out the v4.1 tag, and then made a clean build. Is there an option I need to enable in the cmake build to have the git commit included?
Hi @bryce-carson! I have only made the tag, not declared it as a release, yes. I declare the release on GitHub at the point when the release has been officially announced on slim-discuss. Before that can happen, I need to do things like post the new manual PDFs and the new macOS installer; I'm presently working on those sorts of tasks. :->
I guess you're just talking about adding the AppImage to the list of release items under Assets, like here https://github.com/MesserLab/SLiM/releases/tag/v4.0.1 ? It's a question of where it gets hosted, really, right? Whether GitHub hosts it for us, or you host the file? I think maybe it is better for you to host the file – that way you can get the AppImage posted and tested before I announce the release (ideally)? But I see benefits to putting it on GitHub with all the other assets, too. Hmm. What do you think? If we wanted to include it in the GitHub assets, how would we stage the release exactly?
Hi @bryce-carson! I have only made the tag, not declared it as a release, yes. I declare the release on GitHub at the point when the release has been officially announced on slim-discuss. Before that can happen, I need to do things like post the new manual PDFs and the new macOS installer; I'm presently working on those sorts of tasks. :->
I guess you're just talking about adding the AppImage to the list of release items under Assets, like here https://github.com/MesserLab/SLiM/releases/tag/v4.0.1 ? It's a question of where it gets hosted, really, right? Whether GitHub hosts it for us, or you host the file? I think maybe it is better for you to host the file – that way you can get the AppImage posted and tested before I announce the release (ideally)? But I see benefits to putting it on GitHub with all the other assets, too. Hmm. What do you think? If we wanted to include it in the GitHub assets, how would we stage the release exactly?
It will be hosted on GitHub either way; the question is if you'd prefer it released as an asset on my fork of SLiM, since it's a community binary, or if you'd rather include it as an asset yourself.
For staging, there's nothing to do. I'd just upload the file here in this PR and then you could download it and then upload it as an asset when you make the release.
I'm thinking it might make a little more sense to keep it on my fork, rather than here; I'll go ahead with that. Sometimes thinking aloud helps 🦆.
@bhaller I'm not able to get the Git SHA-1 for some reason. I made a fresh clone of the repository, checked out the v4.1 tag, and then made a clean build. Is there an option I need to enable in the cmake build to have the git commit included?
Hmm, I don't think so? It should be automatic, but it might be fragile for some reason. In the CMakeLists.txt file we have:
# obtain the Git commit SHA-1; see ./cmake/_README.txt and https://stackoverflow.com/a/4318642/2752221
list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/")
include(GetGitRevisionDescription)
get_git_head_revision(GIT_REFSPEC GIT_SHA1)
#message(STATUS "GIT_SHA1 is ${GIT_SHA1}")
You could uncomment that message
line to see whether CMake is seeing the value. The GetGitRevisionDescription
script is in the cmake directory of the SLiM repo. It then finds its way into SLiMgui via the files GitSHA1.h and GitSHA1.cpp.in, which are also in the cmake folder. The _README.txt file in the cmake folder has a brief overview.
The only requirement, I think, is that the repo you're building from be a proper git repo. If you just download the source files, rather than doing a git checkout, the git information is not there, and so it doesn't have a way to get the SHA-1 tag. Could that be the issue?
I'm thinking it might make a little more sense to keep it on my fork, rather than here; I'll go ahead with that. Sometimes thinking aloud helps 🦆.
OK, sounds good. I don't see much of a user-visible difference either way, so I'll follow your lead. We can always revisit this decision in future, for later releases. Either way, it would certainly be nice to have it listed as an asset. It's too bad the AppImage can't include slim
and eidos
also, so that it could be a one-stop solution. But I guess that form of packaging is designed only for apps, not for command-line tools; and as the blurb you put in the manual says, it isn't designed to contain >1 executable, either, I guess.
Hi @bryce-carson. Error:
Merge attempt failed Head branch was modified. Review and try the merge again.
Not sure why; did you start this PR before the final commits for the release? If so, I think you need to rebase it.
Hi @bryce-carson. Error:
Merge attempt failed Head branch was modified. Review and try the merge again.
Not sure why; did you start this PR before the final commits for the release? If so, I think you need to rebase it.
Yeah, I did!
I'll checkout this PR and sort it out ASAP.
@bryce-carson If you're not a git pro (I'm a git noob, personally :->), the easiest thing in this situation is to make a new PR based on the current state of the master branch, and then copy your one modified file in and commit/push. Rebasing always scares me.
@bhaller I rebased the commits and pushed the changes to my fork's master branch. You should be able to merge them now, I believe.
Yep, merged! Thanks; this PR is done, goodbye.
README.md