initial addition of towncrier by RonnyPfannschmidt · Pull Request #2431 · pytest-dev/pytest

RonnyPfannschmidt · 2017-05-23T19:50:30Z

fixes #2390 but still needs to integrate towncrier into release drafting

RonnyPfannschmidt · 2017-05-23T19:53:46Z

@nicoddemus i noticed we manually pass the version everywhere, that kinda prevents automation

coveralls · 2017-05-23T20:17:40Z

Coverage remained the same at 92.347% when pulling c94c19a on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

coveralls · 2017-05-23T20:37:07Z

Coverage remained the same at 92.347% when pulling 1fc257b on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

nicoddemus · 2017-05-23T20:48:49Z

i noticed we manually pass the version everywhere, that kinda prevents automation

What automation do you have in mind? I went with the approach of passing the version to the tasks because part of the tasks was about creating the tag.

If you are thinking about automatically do the entire release after pushing a tag, I personally like the idea, but I think others are not so fan of it.

nicoddemus

Overall excellent changes, thanks @RonnyPfannschmidt!

nicoddemus · 2017-05-23T20:49:26Z

.github/PULL_REQUEST_TEMPLATE.md

+  * name it $issue_id.$type
+  * if you dont have an issue_id change it to the pr id after creating the pr
+  * type is one of removal, feature, bugfix, vendor, doc, trivial
+- [ ] Target: for bbugfix, vendor, doc or trivial fixes, target `master`; for removals or features target `features`;


typo, "bbugfix"

Also, ditto about formatting with monspaced font. 👍

(Sorry if I sound pedantic here, it is just that this will be seen for everyone contributing from now on so I think it pays to be precise and clear)

nicoddemus · 2017-05-23T20:50:44Z

.github/PULL_REQUEST_TEMPLATE.md

+- [ ] Add a new news fragment into the news folder
+  * name it $issue_id.$type
+  * if you dont have an issue_id change it to the pr id after creating the pr
+  * type is one of removal, feature, bugfix, vendor, doc, trivial


I think it is worth formatting removal, feature etc in a monospaced font for emphasis.

nicoddemus · 2017-05-23T20:52:53Z

.github/PULL_REQUEST_TEMPLATE.md


- [ ] Target: for bug or doc fixes, target `master`; for new features, target `features`;
+- [ ] Add a new news fragment into the news folder
+  * name it $issue_id.$type


I would add a simple example:

* name it `$issue_id.$type` (for example `2369.bugfix`)

nicoddemus · 2017-05-23T20:55:44Z

NEWS.rst

@@ -0,0 +1,6 @@
+3.1.1.dev18+g17f6470 (2017-05-23)
+==================
+


I think this doesn't show anything because of the doc change correct? Could you add a few dummy entries for bugfix, features, etc so we can spot any problems with the template?

Shouldn't this file be CHANGELOG.rst given the configuration in pyproject.toml?

nitpicking is graciously accepted and welcomed for changes like this one ^^

nicoddemus · 2017-05-23T20:58:12Z

pyproject.toml

+[tool.towncrier]
+package = "pytest"
+filename = "CHANGELOG.rst"
+directory = "news/"


Small suggestion: since we are calling it CHANGELOG, perhaps this directory should be changes/? I don't feel strongly about it, just a thought.

good thinking, i started by copying from pip and did indeed miss some details

nicoddemus · 2017-05-23T20:59:15Z

tasks/generate.py

+@invoke.task(help={
+    'version': 'version being released',
+})
+def changelog_update(ctx, version):


Perhaps you can have a single changelog command, with a --draft option instead of two commands?

makes sense as well, in that case we should not have a draft option, but a write option in order to opt in instead of out

i think i should invoke the write version from the prerelease step

coveralls · 2017-05-24T06:31:24Z

Coverage remained the same at 92.347% when pulling a49d954 on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

coveralls · 2017-05-24T07:25:46Z

Coverage remained the same at 92.347% when pulling 02b2cde on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

coveralls · 2017-05-24T09:52:44Z

Coverage remained the same at 92.347% when pulling fff5b09 on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

nicoddemus · 2017-05-24T13:25:56Z

CHANGELOG.rst

+    fix problems like typo corrections or such.
+    To add a new change log entry, please see
+    https://pip.pypa.io/en/latest/development/#adding-a-news-entry
+    we named the news folder changelog


Did you plan to run this once with some dummy entries for review only? Then before merging you can discard that commit (like I did with the pre-release tasks).

@nicoddemus im planning to open a own pr against my branch and refer it here

Sounds good! 👍

nicoddemus · 2017-05-24T13:26:25Z

changelog/_template.rst

+
+{% endif %}
+{% if sections[section] %}
+{% for category, val in definitions.items() if category in sections[section] and category != 'trivial' %}


Just to make sure, is this an ordered dict sorted by the order the categories appear in the pyproject.toml file?

my understandign is, that it is ordered

nicoddemus · 2017-05-24T13:26:48Z

tasks/generate.py

+    'version': 'version being released',
+    'write_out': 'write changes to the actial changelog'
+})
+def changelog(ctx, version, write_out=False):


Did you mean to call this from the pre_release tag?

yes, i got sidetracked because it looked like invoke can run dependent taks, bt it cant pass dependent data along with them, so its useless

coveralls · 2017-05-24T14:10:23Z

Coverage remained the same at 92.347% when pulling 721df20 on RonnyPfannschmidt:towncrier into 17f6470 on pytest-dev:master.

RonnyPfannschmidt · 2017-05-24T14:51:14Z

https://github.com/RonnyPfannschmidt/pytest/pull/1/files

nicoddemus · 2017-05-25T20:11:07Z

Just realized this: I think we should run rst-lint on the news snippets, what do you think?

nicoddemus · 2017-05-30T02:38:11Z

Any updates on this? Looking forward to have this live! 😀

RonnyPfannschmidt · 2017-05-30T07:28:06Z

@nicoddemus its working now - see https://github.com/RonnyPfannschmidt/pytest/pull/1/files

nicoddemus · 2017-05-30T12:07:38Z

.github/PULL_REQUEST_TEMPLATE.md

+- [ ] Add a new news fragment into the changelog folder
+  * name it `$issue_id.$type` for example (588.bug)
+  * if you dont have an issue_id change it to the pr id after creating the pr
+  * type is one of `removal`, `feature`, `bugfix`, `vendor`, `doc`, `trivial`


Final nitpick before merging, I think we should add:

* Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

To ensure a consistent-looking changelog.

Also the bullet points in here themselves could use the same treatment. 😉

nicoddemus · 2017-05-30T12:08:35Z

Except for my last nitpick regarding the text in the issue template, this LGTM and ready for merging. Would you like to rebase before merging?

Excited to see this live. 👍

nicoddemus · 2017-05-30T13:57:48Z

Awesome. Will merge as soon as CI passes.

coveralls · 2017-05-30T14:22:47Z

Coverage remained the same at 92.237% when pulling b74c626 on RonnyPfannschmidt:towncrier into 6117930 on pytest-dev:master.

RonnyPfannschmidt requested review from The-Compiler and nicoddemus May 23, 2017 19:52

nicoddemus reviewed May 23, 2017

View reviewed changes

nicoddemus reviewed May 24, 2017

View reviewed changes

This was referenced May 25, 2017

Fix unicode issue while running doctests in Python 2 #2438

Merged

Correct warnings that contains Unicode message #2437

Merged

RonnyPfannschmidt changed the title ~~[dontmerge] initial addition of towncrier~~ initial addition of towncrier May 30, 2017

nicoddemus reviewed May 30, 2017

View reviewed changes

switch changelog management to towncrier

b74c626

RonnyPfannschmidt force-pushed the towncrier branch from 05620b4 to b74c626 Compare May 30, 2017 13:54

nicoddemus approved these changes May 30, 2017

View reviewed changes

nicoddemus merged commit ee0844d into pytest-dev:master May 30, 2017

userzimmermann mentioned this pull request Jun 6, 2017

Release Team Leader Application: Max Hahn coala/teams#39

Closed

RonnyPfannschmidt deleted the towncrier branch June 11, 2017 13:43

		@@ -0,0 +1,6 @@
		3.1.1.dev18+g17f6470 (2017-05-23)
		==================

Uh oh!

Conversation

RonnyPfannschmidt commented May 23, 2017

Uh oh!

RonnyPfannschmidt commented May 23, 2017

Uh oh!

coveralls commented May 23, 2017

Uh oh!

coveralls commented May 23, 2017

Uh oh!

nicoddemus commented May 23, 2017

Uh oh!

nicoddemus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 24, 2017

Uh oh!

coveralls commented May 24, 2017

Uh oh!

coveralls commented May 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coveralls commented May 24, 2017

Uh oh!

RonnyPfannschmidt commented May 24, 2017

Uh oh!

nicoddemus commented May 25, 2017

Uh oh!

nicoddemus commented May 30, 2017

Uh oh!

RonnyPfannschmidt commented May 30, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicoddemus commented May 30, 2017

Uh oh!

nicoddemus commented May 30, 2017

Uh oh!

coveralls commented May 30, 2017

Uh oh!

Reviewers

Assignees

Labels