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

chore: Add templates for PRs and automated release notes #3381

Merged
merged 10 commits into from
Mar 30, 2024
Merged

Conversation

joelostblom
Copy link
Contributor

@joelostblom joelostblom commented Mar 23, 2024

Continuation of #3339. This is an example of a PR template we could use to encourage contributors to use conventional commit style for the title. In addition to the conventional commit homepage, angular has a short description of what each title prefix indicates.

To be sorted into specific sections in the automated GitHub release notes, we would also need to label each PR. I could not find a way to automate this. The closest is this action, but it is outdated https://github.com/marketplace/actions/conventional-release-labels. I don't think it is a big deal to add labels manually and if we forget a label the PR will still be included in the automated release notes draft, just not under the correct section, but it's easy to move it manually.

I think the relevant title prefixes and labels would be the following:

Title prefix Label
fix bug
feat,perf enhancement
docs documentation
chore,build,refactor,style,test,ci,revert maintenance

Edit: Let's put it under enhancement I'm unsure if perf should go under maintenance. It seems like it could also be an enhancement, although it is not a new "feature" per se... Maybe we should call the label feature instead of enhancement? Open to suggestions

As @binste pointed out, we could also add an automated check for the PR title, e.g. via https://github.com/marketplace/actions/semantic-pull-request which VL uses. Edit: I added this in 12fbb62

Copy link
Contributor

@binste binste left a comment

Choose a reason for hiding this comment

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

Thanks for pulling this together @joelostblom! I really like that we can add some structure to PR titles and make the release process easier.

I agree with you that it's totally fine if the automatically generated release notes still require us to sort the PR's which were not labelled. That's a quick thing to do but it's also great to already get the automatic categorization with release.yml, I wasn't aware of this feature :)

I added some minor comments. Regarding perf, mild preference to put it into Enhancements. It's something which makes the library better, I think Maintenance should be things which the users ideally don't notice.

@@ -0,0 +1,9 @@
Thanks for contributing to Altair!
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the text! Could we wrap it in <!-- Here the text --> so that we don't even have to delete the message before submitting the PR? See how Ibis handles this: https://github.com/raw/ibis-project/ibis/main/.github/PULL_REQUEST_TEMPLATE.md

I think I've also seen this in other repos as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I have seen this in other repos too, however I'm a bit split on whether it is preferable. The reason I kept it as uncommented text here is so that it can be read in the rich preview; I find that the links makes the comment cluttered and hard to parse.

For the issues, the answer is to use "Issue forms" instead of the regular templates. This renders links and formatting nicely while not including it in the actual issue. In my latest commit I switched both our issue templates over, you can see a demo in this other repo I made.

image

image

Unfortunately, forms don't exist for PR .

.github/ISSUE_TEMPLATE/feature-request.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug-report.md Outdated Show resolved Hide resolved
.github/release.yml Outdated Show resolved Hide resolved
joelostblom and others added 3 commits March 29, 2024 08:36
Co-authored-by: Stefan Binder <binder_stefan@outlook.com>
@joelostblom joelostblom requested a review from binste March 29, 2024 22:49
@joelostblom
Copy link
Contributor Author

Thanks for the review @binste. I agree perf is better suited for enhancement and updated the table above. I replied to your inline comment regarding formatting for the templates.

Copy link
Contributor

@binste binste left a comment

Choose a reason for hiding this comment

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

The issue templates are really nice! And I see your point regarding the readability of the comments. Thanks!

@binste binste merged commit 51aa57f into main Mar 30, 2024
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants