10000 ENH: use towncrier to build the release note by mattip · Pull Request #13908 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

ENH: use towncrier to build the release note #13908

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 24, 2019
Merged

ENH: use towncrier to build the release note #13908

merged 1 commit into from
Jul 24, 2019

Conversation

mattip
Copy link
Member
@mattip mattip commented Jul 3, 2019 •
edited
Loading

Add infrastructure and instructions to use towncrier to build the release note from a directory of fragments.

python -mtowncrier --draft (once there are some fragments in ./changelog) will preview the current release note status.

Running python -mtowncrier will overwrite ./doc/release/latest-note.rst, delete all the fragments from ./changelog, and stage the changes for a git commit.

A fragment is a file in ./changelog with a name like 139999.changes.rst with a title line and content like

Use ``towncrier`` to build the release note
-------------------------------------------
Use ``towncrier`` to build the release note out of fragments in a directory

@seberg
Copy link
Member
seberg commented Jul 5, 2019

I am wondering if we can do a tiny CI check to avoid bad file naming?

seberg
seberg approved these changes Jul 5, 2019
View reviewed changes
Copy link
Member
@seberg seberg left a comment

Choose a reason for hiding this comment

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

Looks good to me. Lets not worry too much about some extra bells and whistles. It would be good to set this up to make it easier to merge some PRs and other things can be added later. @charris does this seem good to you?

doc/RELEASE_WALKTHROUGH.rst.txt Outdated
@@ -38,6 +38,10 @@ to the maintenance branch, and later will be forward ported to master.
Finish the Release Note
-----------------------

.. note:

This has changed now that we use ``towncrier``
Copy link
Member

Choose a reason for hiding this comment

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

Might be a bit cryptic without information on where to look?

Copy link
Member Author

Choose a reason for hiding this comment

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

adding the path to the README.rst

shoyer
shoyer reviewed Jul 5, 2019
View reviewed changes
Copy link
Member
@shoyer shoyer left a comment

Choose a reason for hiding this comment

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

This should definitely make for a better contributor workflow :)

changelog/README.rst Outdated
If you are unsure what pull request type to use, don't hesitate to ask in your
PR.

Note that the ``towncrier`` tool will automatically reflow your text, so it
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is true for our template, so this should probably be removed -- you removed the automatic insertion of list items from the pytest template.

Copy link
Member Author

Choose a reason for hiding this comment

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

removing

@eric-wieser eric-wieser mentioned this pull request Jul 8, 2019
Merged
@mattip
Copy link
Member Author
mattip commented Jul 14, 2019

It seems towncrier requires a pyproject.toml file, but the default build-system section causes pip install . tries to build numpy without cython.

mattip
mattip commented Jul 14, 2019
View reviewed changes
setup.py Outdated
@@ -364,7 +364,7 @@ def parse_setuppy_commands():


def setup_package():
src_path = os.path.dirname(os.path.abspath(sys.argv[0]))
src_path = os.path.dirname(os.path.abspath(__file__))
Copy link
Member Author

Choose a reason for hiding this comment

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

When using pyproject.yaml, setup.py is now called via exec(compile(code, __file__, 'exec'), locals()) in python pip/_vendor/pep517/_in_process.py get_requires_for_build_wheel /tmp/tmp6g6mz9bq. Since it is not called directly on the command line in a subprocess but via exec, sys.argv[0] is no longer ./setup.py.

Copy link
Member

Choose a reason for hiding this comment

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

yeah, just tracked down this too, but did not manage to quite get to the bottom of everything, maybe we can fix it tomorrow, or push towncrier to give an alternative toml file... (I commented on an issue there)

Copy link
Member

Choose a reason for hiding this comment

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

This change looks harmless to me - functions in setup.py aren't public API, and any use-case involving symlinks to setup.py seems incredibly contrived

eric-wieser
eric-wieser reviewed Jul 14, 2019
View reviewed changes
pyproject.toml
@@ -0,0 +1,60 @@
[build-system]
# Minimum requirements for the build system to execute.
requires = ["setuptools", "wheel", "cython"] # PEP 508 specification
Copy link
Member

Choose a reason for hiding this comment

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

It's tempting to do the PEP 508 stuff in a separate PR, just to give it a little more visibility. This looks harmless, but it might be good to seed discussion about any further PEP508 changes

@mattip mattip mentioned this pull request Jul 14, 2019
Merged
@mattip
Copy link
Member Author
mattip commented Jul 16, 2019

Adding the pyproject.toml file seemed non-controversial on the mailing list. Can this go in as-is?

@mattip mattip mentioned this pull request Jul 17, 2019
Merged
@seberg
Copy link
Member
seberg commented Jul 17, 2019

I have opened twisted/towncrier#156 which adds --pyproject to pass in a fixed toml file. That one probably still needs tests so need to figure it out, but it is of course very easy to add to towncrier.

@mattip mattip mentioned this pull request Jul 18, 2019
Merged
@mattip
Copy link
Member Author
mattip commented Jul 18, 2019

When #14053 goes in this will need a rebase

@seberg
Copy link
Member
seberg commented Jul 23, 2019

OK, rebased and squashed this. Will likely merge soon.

@seberg seberg self-requested a review July 23, 2019 20:34
@seberg
Copy link
Member
seberg commented Jul 24, 2019 •
edited
Loading

OK, I will merge this. happy to revert and clean up everything if someone is unhappy with that (sorry in that case). But there are a few PRs that I would like to get going, and they need release notes, so being unclear about how to write them best right now is not helpful.

(EDIT: last push only removed bogus blank line from setup.py)

@seberg seberg merged commit 2d0f2dd into numpy:master Jul 24, 2019
@seberg
Copy link
Member
seberg commented Jul 24, 2019

Ok, I noticed that there is at least one typo and the final release notes are indented a bit funnily (template issue probably). These are small things and I will probably create a quick update PR soon.

@seberg
Copy link
Member
seberg commented Jul 25, 2019

Hmmm, I am a bit disappointed. It seems to me that towncrier expects release notes to always come in a bullet point format. But we often have longer text and currently use a subheading format. And right now, I think I would prefer to keep it like that. I think I will look into pushing some fixes for that upstream...

Does not need to stop us using it for the moment though, at least we cannot possibly run into issues with merge conflicts...

@mattip mattip deleted the towncrier branch October 13, 2020 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eric-wieser eric-wieser eric-wieser left review comments

@shoyer shoyer shoyer left review comments

@seberg seberg Awaiting requested review from seberg

Assignees
No one assigned
Labels
04 - Documentation component: build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mattip @seberg @eric-wieser @shoyer
0