make-release.py script #2393
make-release.py script #2393
Conversation
…ttests [unittests fail].
…e 0 digits in major/minor/patch.
…uild numbering w/ unittests.
…release-process.md. [Manually tested.]
nathan-at-least
requested review from
daira,
str4d,
arcalinea,
ageis and
bitcartel
The release process used to be performed on a branch, which was then used to create a release PR. Is that not the case anymore? |
|
@bitcartel looks like the script itself creates the release branch, meaning it has to start from master. |
|
For reference, if the branch already exists, the script exists with the following message: |
|
On completion, the git commit messages don't contain any version or release information such as 1.0.9. e.g. |
|
@bitcartel Good idea. I think those git commit messages could easily say 'for release X.Y.Z'. |
|
Running on non- |
doc/release-process.md
Outdated
|
|
||
| $ git add ./doc/authors.md ./doc/release-notes/release-notes-$ZCASH_RELEASE.md | ||
| $ ./zcutil/make-release.py v1.0.8-1 v1.0.9 |
There was a problem hiding this comment.
Example has argument order reversed relative to the specified command.
zcutil/make-release.py
Outdated
| initialize_git(release) | ||
| patch_version_in_files(release, releaseprev) | ||
| patch_release_height(releaseheight) | ||
| commit('Versioning changes.') |
There was a problem hiding this comment.
Would be useful for at least one of these commit messages to contain release.
zcutil/make-release.py
Outdated
| def verify_releaseprev_tag(releaseprev): | ||
| candidates = [] | ||
| for tag in sh_out('git', 'tag', '--list').splitlines(): | ||
| if tag.startswith('v1'): # Ignore v0.* bitcoin tags and other stuff. |
There was a problem hiding this comment.
We'll need to change this before Sapling. Perhaps would be better to search for v0 and ignore? Or search for v, require second char to be an int, and check for val > 0?
|
|
||
|
|
||
| @phase('Patching release height for auto-senescence.') | ||
| def patch_release_height(releaseheight): |
There was a problem hiding this comment.
This doesn't patch WEEKS_UNTIL_DEPRECATION, which may also need to change.
There was a problem hiding this comment.
WEEKS_UNTIL_DEPRECATION should only change when the policy changes, in which case I believe that should be in a PR prior to the release PR.
There was a problem hiding this comment.
The deprecation policy is not "18 weeks after a release", but "the third Tuesday four months after a release", which is a variable duration.
There was a problem hiding this comment.
The 'auto-sensence' feature design, as I imagined it, is 'sometime after a release is deprecated it will automatically halt'. The 'sometime after' was deliberate wording to let us rely on block heights and conservative estimates even though the deprecation policy is in calendar time. In terms of protocol planning we can rely on block heights. Humans have a calendar deadline even though it's a conservative fiction.
There was a problem hiding this comment.
Oh, I reread https://z.cash/blog/release-cycle-and-lifetimes.html and I don't see my careful wording there. I think it got lost in the edits. Anyway, I don't think anyone would be unpleasantly surprised if the auto-senescence cutoff is always sometime after the policy date, and I prefer to keep the code and block heights simpler. Does that seem acceptable?
There was a problem hiding this comment.
That's fine, but then we should just say that a release is deprecated 18 weeks after it is released. If that is the official policy, then there are no inconsistencies beyond block-height vs. calendar-time discrepancies. But with the current policy, we have calendar-time vs. calendar-policy discrepancies, which IMHO are more serious.
There was a problem hiding this comment.
More concretely, we have not confirmed that 18 weeks is always "sometime after" the third Tuesday four months after release.
There was a problem hiding this comment.
Hm, in the case I think we should say the policy is 16 weeks after release, and autosenescence kicks in approximately 18 weeks after release.
| with PathPatcher(path) as (inf, outf): | ||
| outf.write(inf.readline()) | ||
|
|
||
| secondline = inf.readline() |
There was a problem hiding this comment.
Fine for now, but more fragile than the assumption that the README change happens on the first line.
zcutil/make-release.py
Outdated
| 'REVISION': release.patch, | ||
| 'BUILD': release.build, | ||
| 'IS_RELEASE': ( | ||
| 'false' if release.betarc == 'beta' else 'true' |
There was a problem hiding this comment.
This should instead be release.build < 50 (at least for now).
…ect arguments and order.
…ion parser schema.
|
I addressed all feedback from @str4d and @bitcartel either by making changes or clarifying the intent. |
|
@zkbot r+ |
|
📌 Commit 88bf7eb has been approved by |
…han-at-least make-release.py script ref #2391 This is a new `make-release.py` script which automates creation of the 'release PR' branch. It has partial unittest coverage (large around version parsing/sorting/serializing) and always runs unittests prior to doing actual work. Most of the testing was done manually by using the ``--repo`` arg on a test repo, then reseting its state each time I needed a new test (to get around git checks). There is no other 'dry run' functionality.
ref #2391
This is a new
make-release.pyscript which automates creation of the 'release PR' branch.It has partial unittest coverage (large around version parsing/sorting/serializing) and always runs unittests prior to doing actual work. Most of the testing was done manually by using the
--repoarg on a test repo, then reseting its state each time I needed a new test (to get around git checks). There is no other 'dry run' functionality.