-
-
Notifications
You must be signed in to change notification settings - Fork 409
version: retrieve commit hash for git worktrees #2586
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
Conversation
7a0ee69 to
5ffe1a7
Compare
Well it's not in
It's the same issue I described in a couple of my PRs, and is probably because you haven't reinstalled/ To me, this change is low-stakes and could be shipped in 8.0.0 while I still lack the time (😩) to work on release notes. (It's just nuts how much stuff has kept coming up for me this month! I thought I had everything planned out for $work etc. but noooo.) |
Huh, I had an old
Maybe, but let's defer it anyway IMO. |
Exirel
left a comment
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.
That's looking good, feature wise. Code wise, it's mostly style related.
I didn't put a comment on the f variable, because it isn't important, however I tend to use fd (for file descriptor) instead of just f. In general, I prefer more expressive code and names. I don't think reducing the character count on variable names that are less than 10 characters matters.
|
I don't feel comfortable having this in 8.0.1. I'm sure this can wait Sopel 8.1 - beside, it should be useful mostly when working with an unstable version from git, so something that would fit nicely in master and unreleased for now. |
Agreed |
|
A good squash would make the history more readable, other than that it's good to go! |
bc86322 to
bc058da
Compare
Co-authored-by: Florian Strzelecki <[email protected]>
bc058da to
2b3ae76
Compare
|
I'm impressed that there are no merge conflicts considering the age of this PR! Squashed down to 2 commits, I figure it makes sense to retain a separate commit for renaming the existing function. |
Description
Tin and personal wish fulfillment. With this changeset, the
versioncommand will report commit information if Sopel is running from agit-worktree.The current revision of the
versionplugin only supports reading commit information when the host Sopel is running inside of a 'real' git repository. I find it convenient to run Sopel from a worktree for a "near to development" copy, and it's nice to still have the commit information in this use-case.Checklist
make qa(runsmake lintandmake test)mypybecausetypes-urllib3was not installed. I'm not sure if that's missing fromdev-requirements.txtor if my local environment was in a weird state.test_example_suggest_*caused by content decoding errors. I only vaguely recognize the issue, but I'm confident it's not this changeset's fault.