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

REL: v1.4.0 #496

Merged
merged 2 commits into from
Jun 11, 2020
Merged

REL: v1.4.0 #496

merged 2 commits into from
Jun 11, 2020

Conversation

franklin-feingold
Copy link
Collaborator

This PR coordinates with releasing Common Derivatives into BIDS!

Following our release protocol this will be merged in 1 week pending further discussions.

@nicholst
Copy link
Collaborator

nicholst commented Jun 12, 2020

Hi @franklin-feingold, It's a minor issue, but I thought part of the release protocol was tidying up the PR names so that the auto-changelog is clean and professional. I mean, it's minor, but if I was scanning the change notes and a saw Enh/prov I'd have no clue what this meant; also, some are missing the FIX/ENH/etc tags.

How do we fix that now? I assume you don't just lodge a PR to change the changelog?

@effigies
Copy link
Collaborator

effigies commented Jun 12, 2020

Rename the PR and it will be fixed in future changelogs.

That's an annoying one, because it wasn't merged into master, so I'd think it should be excluded...

@nicholst
Copy link
Collaborator

Sorry... I can rename PR's when I see it....

But, um what's the "annoying one"? Satra's Enh/prov?

@satra
Copy link
Collaborator

satra commented Jun 12, 2020

does the automation script only check closed? or merged?

@franklin-feingold
Copy link
Collaborator Author

does the automation script only check closed? or merged?

The script checks for merged PRs. We don't have additional options turned on for capturing pr's merged into master. Looking the options available I see a few different approaches.

  • We could add labels to the pr's going into one of our branches. This will make it more clear when scanning the pr's which are going into master and which are merging into one of our branches. Although this strategy breaks down if we are giving slightly too granular labels (e.g. BEP vs BEP028). Each new BEP label would need to be added to our exclusion list.

  • They have a release-branch flag that appears to limit to pr's to our master branch. I'll have to test it more in the coming days but this option may resolve this issue nicely.

@nicholst
Copy link
Collaborator

OK, well I'm not sure if I understand if the proposed solutions are forward-looking or also account for retrospective changes. FWIW, I've renamed the offending PRs:

to

Hope that's OK/correct.

@satra
Copy link
Collaborator

satra commented Jun 13, 2020

@franklin-feingold - i think i see what the reason is the provenance PR that was merged, was not merged into master. it was merged into a separate branch.

this query on PRs will reveal the set of PRs merged into master since the last release:
is:pr is:merged base:master merged:>=2020-04-14

and if you wanted to do additional tracking, you can indeed setup milestone labels.

@franklin-feingold
Copy link
Collaborator Author

@satra sounds good thank you! I'll dig further and see what I can put together

@nicholst thank you for renaming - looks good! Yep our generator accounts for retrospective changes too because the changelog is generated at each merged PR

@sappelhoff sappelhoff deleted the rel/1.4.0 branch October 12, 2020 09:07
@sappelhoff sappelhoff added the exclude-from-changelog This item will not feature in the automatically generated changelog label Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude-from-changelog This item will not feature in the automatically generated changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants