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

Add towncrier #2873

Merged
merged 12 commits into from Jul 17, 2019

Conversation

@mikeshardmind
Copy link
Member

commented Jul 14, 2019

Type

  • Bugfix
  • Enhancement
  • New feature

Description of the changes

  • Adds towncrier as our changelog system.
  • Updates our contributor guidelines for this.

Resolves #2872

@mikeshardmind mikeshardmind requested a review from Tobotimus Jul 14, 2019
@mikeshardmind mikeshardmind requested a review from Twentysix26 as a code owner Jul 14, 2019
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Jul 14, 2019
Copy link
Member

left a comment

Just a few things:

CONTRIBUTING.md

Probably need a few extra full stops at the end of sentences in section 4.7. Also, you might need to update the table of contents at the top with the new section + the updated section numbers.

Something I wondered about was what the filename should look like if the PR doesn't have a corresponding issue, or what the PR author should do if they're closing multiple issues at once (multiple changelog files?).

Requirements

I might need to make this clearer for further use, but to add a dependency:

  • Add it in tools/primary_deps.ini under the appropriate extra
  • Run make bumpdeps if you're on Linux and copy across the section you updated to setup.cfg

In this case, since towncrier is in one of the extras, it doesn't need its own entry in dev-requirements.txt.

New make recipe

If make checkchangelog must be run in tox -e docs, it should only really be run on Travis. But there are two options here I suppose:

  1. Include make checkchangelog on local tox -e docs runs as well, but modify tools/check_changelog_entries.sh so it checks against the V3/develop branch of this repository, and it should inspect the URLs of the remotes to figure that out (it's the only way I think it can reliably do that)
  2. Separate it into a new tox environment and have one of the travis jobs (probably the style one because it's currently the fastest) run both at once like TOXENV=style,changelog. You might want to give towncrier its own extra in primary_deps.ini and setup.cfg if you do this.
@mikeshardmind

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

Just a few things:

CONTRIBUTING.md

Something I wondered about was what the filename should look like if the PR doesn't have a corresponding issue, or what the PR author should do if they're closing multiple issues at once (multiple changelog files?).

If it closes multiple issues, make a file for each issue it closes.

If there's no corresponding issue, the PR can be opened and the PR number used instead. (requires the towncrier entry to be created after the PR is opened)

Additionally, if there need to be multiple lines in the same section for the same issue or PR, this can be done as well.

The full spec for a filename with the current settings is

basepath_or_section_folder/issue_or_pr_number.change_type(.count).rst

where (.count) is an optional part for including multiple lines for the name issue + change type

I'll go ahead and make some additional changes here in a bit. to address the other issues. I'm actually going to remove the changelog check from the makefile and have tox invoke the bash script directly while on travis

@mikeshardmind mikeshardmind force-pushed the mikeshardmind:towncrier branch from 4a116c5 to efd07b4 Jul 16, 2019
@mikeshardmind

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

I've made the fragment check only run on Travis for now. (and fixed the depedency bit)

I've chosen to leave it in the docs section as changelogs as used here really are documentation.

I'll go back and improve some of the scripting here to make it more useful locally later on, but this is really just for a check existing to help support process.

@mikeshardmind mikeshardmind requested a review from Tobotimus Jul 16, 2019
@mikeshardmind mikeshardmind dismissed Tobotimus’s stale review Jul 16, 2019

(See changes)

@mikeshardmind

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

Sec, I just noticed the updated check is not running properly.

@mikeshardmind

This comment has been minimized.

Copy link
Member Author

commented Jul 16, 2019

Went ahead and made the changelog fragment check attempt to use an env var RED_REMOTE, defaulting back to origin if it isn't set.

Making this more useful probably means there should be a corresponding windows entry for the makefile at some point, but my priority on this is getting this working for Travis.

@jack1142

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

4.8 and 4.9 anchors have a number too low so they don't work

* [4.8 How To Report A Bug](#47-how-to-report-a-bug)
* [4.9 How To Suggest A Feature Or Enhancement](#48-how-to-suggest-a-feature-or-enhancement)

@jack1142

This comment has been minimized.

Copy link
Contributor

commented Jul 16, 2019

Oh, and the file format has a one dot too much:
Instead of issuenumber.changetype.(.count).rst it should be issuenumber.changetype(.count).rst

@Tobotimus Tobotimus merged commit d4b6fde into Cog-Creators:V3/develop Jul 17, 2019
1 check passed
1 check passed
Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.