-
Notifications
You must be signed in to change notification settings - Fork 16
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
Integrate bump2version with auto #96
Closed
Closed
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[bumpversion] | ||
current_version = 0.8.4 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh and if we need to duplicate current version here it would ruin all point of |
||
commit = True | ||
message = [skip ci] Bump version: {current_version} → {new_version} | ||
tag = False | ||
|
||
[bumpversion:file:datalad_crawler/version.py] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
__version__ = '0.8.2' | ||
__version__ = '0.8.4' |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
what is the benefit of bringing in bump2version over some simple
echo "__version__ = \"$ARG_0\"" >| datalad_crawler/version.py
ifARG_0
is the one containing version here?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.
$ARG_0
doesn't contain the new version; it contains the name of the version level that is being incremented ("major", "minor", or "patch"), which is perfect forbump2version
.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.
and auto itself has no functionality to bump or report back the version? I thought it is the one which "mints" a new version to be, since otherwise I do not see e.g. how in dandi-cli version is decided upon -- we rely on versioneer, and I thought that one just uses the git tag, but then something mints the git tag (so the version). could you please digest the workflow here for me a bit more?
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.
auto can and does determine a new version, but (last time I checked) its
$ARG_0
payload does not include that information; it only contains the changed version level. auto determines the version level based on the tags on the PRs since the last release, increments the version of the last release appropriately, generates & commits a changelog, runs theafterChangelog
hook here to increment__version__
in the source code and commit the change, and then it tags the latest commit with the new version, pushes the tag, and creates a GitHub release.Even if auto did report the actual version, the
afterChangelog
command would still need to include agit commit
command after updatingversion.py
, whereasbump2version
can do all of that at once.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.
THANK YOU. Now it feels less magical. I would still prefer to avoid bump2version: even if both
auto
andbump2version
use the similar logic for bumping a version, that is two different logics and might result in different results. Also still feels too heavy for such a tiny job.If I read https://github.com/intuit/auto/blob/main/packages/core/src/auto.ts#L1880 code correctly,
ARG_4
might be thecurrentVersion
which if I get it right - is the new versionauto
decided upon. I think it is Ok to do a commit manually BUT ideally it should even be done along with the commit to the changelog: and looking at https://github.com/intuit/auto/blob/main/packages/core/src/auto.ts#L1896 it seems thatbeforeCommitChangelog
should be the hook to use to produce desired version.py (edit: and probably need to stage it withgit add
). So could we 'tap' into that hook?Even more ideally -- name of the package should be figured out automagically and it is possible!:
This would allow for the release workflow to be easily reused across multiple packages.
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.
No, as I said, the env dumps were generated when releasing version 0.3.2.
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.
See also: this 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.
ah, that comment did not appear whenever I made my comment. gotcha.
it is a bit odd then that
auto
does not provide updated version - what is the point of having currentVersion and lastRelease to match?Is there may be some
auto
command which would print an updated 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.
worse comes to worse, it could as well be
to pick up the version, in
beforeCommitChangelog.env
right?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.
may be with some care/test with grep that obtained value is a semver in syntax