Skip to content
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

Release instructions #317

Merged
merged 8 commits into from
Feb 5, 2021

Conversation

altendky
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Jan 17, 2021

Codecov Report

Merging #317 (bfb7611) into master (00f19b4) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #317   +/-   ##
=======================================
  Coverage   96.78%   96.78%           
=======================================
  Files          20       20           
  Lines        1183     1183           
  Branches      106      106           
=======================================
  Hits         1145     1145           
  Misses         20       20           
  Partials       18       18           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00f19b4...bfb7611. Read the comment docs.

CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated

- Once the final release has been made, leave the approving review intact and merge the PR.

# TODO: should we have an indicator on the version for interrim development or just leave the last release version intact?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably ought to just leave this for a separate activity I guess. It isn't needed to do a release and isn't presently being done at all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I think that we should have a placeholder like. "No changes since last release."

For my project, I do a stupid think and replace the release candidate in the release notes with the final note.
But this erases the project history and I think it's bad practice.

In my fork of towncrier, If I run towncrier with a new version and no news fragments I get an em

But we can have something like this for the default template

{% set underline = "^" %}
{% set section = sections[''] %}
{% set empty_release = True %}

{% for category, val in definitions.iteritems()%}
{% if definitions[category]['showcontent'] and section[category]|length > 0 %}
{{ definitions[category]['name'] }}
{{ underline * definitions[category]['name']|length }}

{% for text, values in section[category]|dictsort(by='value') %}
* {{ text }} {{ values|sort|join(', ') }}
{% set empty_release = False %}
{% endfor %}


{% endif %}
{% endfor %}
{% if empty_release %}
No changes since the last release.
{% endif %}

To produce something like


Version 4.8.0, 2021-02-01
-------------------------


No changes since the last release.


Version 4.8.0rc1, 2020-11-19
-------------------------


New Features
^^^^^^^^^^^^

* The embedded OpenSSL libraries used on Windows, macOS, and generic Linux were
  updated to version 1.1.1h. [#5496]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the project version, unrelated to generating release notes. Rather, if I checkout and install an arbitrary commit of towncrier I will get one claiming to be the last release version since after doing a release the version is just left alone.

Despite having numerous changes since the release of 19.2, the latest commit on master still claims to be 19.2.

https://github.com/twisted/towncrier/blame/00f19b4d24ad5dfd57124f7c93f9ba2107408f8c/src/towncrier/_version.py#L10

Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just initial comments

CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a very good start. Thanks. I left my comments.
You are the boss so feel free to do it as you like :)

Copy link
Member Author

@altendky altendky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep master up to date, perhaps the release branch should be merged back into master not only at the end but also after each pre-release? Any pre-release ought to exist in the history even if something drastic is changed about it so I'm not sure in what case a released commit should not exist in master or why it's merging should wait. If this is good, the existing post-final-release step to merge would be shifted to apply after every release.

CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated Show resolved Hide resolved
CONTRIBUTING.rst Outdated

- Once the final release has been made, leave the approving review intact and merge the PR.

# TODO: should we have an indicator on the version for interrim development or just leave the last release version intact?
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was referring to the project version, unrelated to generating release notes. Rather, if I checkout and install an arbitrary commit of towncrier I will get one claiming to be the last release version since after doing a release the version is just left alone.

Despite having numerous changes since the release of 19.2, the latest commit on master still claims to be 19.2.

https://github.com/twisted/towncrier/blame/00f19b4d24ad5dfd57124f7c93f9ba2107408f8c/src/towncrier/_version.py#L10

Co-authored-by: Adi Roiban <adiroiban@gmail.com>
@adiroiban
Copy link
Member

To keep master up to date, perhaps the release branch should be merged back into master not only at the end but also after each pre-release?

I would say it's not worth the trouble.
But it's your call :)

I was referring to the project version, unrelated to generating release notes. Rather, if I checkout and install an arbitrary commit of towncrier I will get one claiming to be the last release version since after doing a release the version is just left alone.

Sorry for missing that.

Yes. I think that after the public release there should another step to update the release to +1 and + .dev0

so if 21.1.0 was release, after the release the release branch is updated with 21.1.1dev0 and merged into master.

For a 2.1.2 release... it will be updated to 2.1.3dev0

@altendky
Copy link
Member Author

altendky commented Feb 4, 2021

this is getting painful... not complaining about your reviews, just the detail and growth of the process and a minimum of triplicate reviews and dismissing reviews and... the review dismissals make me wonder if it would be simpler to treat each sub-release via a separate PR against the release branch. sounds complicated, also sounds simpler since you wouldn't dismiss reviews. anyways. :|

@altendky altendky marked this pull request as ready for review February 4, 2021 03:07
@altendky altendky requested a review from adiroiban February 4, 2021 03:07

- Return to the towncrier build step and continue.

- If the final release has been completed, continue below.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe keep it simple.

I don't think that we need to go into the edit src/towncrier/_version.py as by this point people should know how do update the version.

Suggested change
- If the final release has been completed, continue below.
- If the final release has been completed, mark current version as development `Version('towncrier', 19, 9, 1, dev=0)`` and merge it without requesting another review.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll drop the review requirement but I don't feel that not stating what to do makes the instructions simpler. Seems mostly just inconsistent with the rest of the instructions.


- Return to the towncrier build step and continue.

- If the final release has been completed, continue below.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll drop the review requirement but I don't feel that not stating what to do makes the instructions simpler. Seems mostly just inconsistent with the rest of the instructions.

CONTRIBUTING.rst Outdated Show resolved Hide resolved
Copy link
Member

@adiroiban adiroiban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good. Thanks!

I think that having a single PR is easier. I guess that we can give it a try and see if things get ugly.. .then the process can be changed with a PR for each release and separate PR for each release candidates.

I think the title of this PR can be updated and this PR merged .

Looking forward to the next release where we can practice what is documented here.

Many thanks again for putting this together.

@altendky altendky changed the title Preliminary release instructions Release instructions Feb 5, 2021
@altendky
Copy link
Member Author

altendky commented Feb 5, 2021

Thanks for seeing it through with the reviews.

@altendky altendky merged commit 7f20910 into twisted:master Feb 5, 2021
@altendky altendky deleted the release_process_documentation branch February 5, 2021 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants