-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Skip changelog generation when changelog type is none #13216
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
Skip changelog generation when changelog type is none #13216
Conversation
Hello! I am a robot. Tests will require approval from a repository maintainer to run. @trodge, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 1 week. Please take a look! Use the label |
@trodge This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 1 week. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 3 weeks. Please take a look! Use the label |
@GoogleCloudPlatform/terraform-team @trodge This PR has been waiting for review for 4 weeks. Please take a look! Use the label |
378061c
Thanks @trodge. were you able to find a way to test? from what I can see, this generated nothing downstream, and hashicorp/terraform-provider-google#22229, which came after mine, did seem to generate one, so hopefully it's working(ish)? I noticed some PRs have multiple release notes, but I assume it wouldn't be expected to have a "none" type mixed with other / real types? Not sure if there could be some kind of corner case that this would run up against. Is it worth me making a PR to remove the empty changelog entries as well (directly against tpg / tpg-beta repos)? |
Could use some help in terms of doing some functional testing on this and / or suggesting a way I could add some test coverage. But basically, the idea here is to skip generating a changelog entry when the changelog type is
none
Mostly, this should prevent adding an extra changelog entry (whether or not any other changes are generated), avoiding creating extra noise in the downstream repos to create a bunch of empty files.
Separately, I could make a PR to clean up the empty changelog files in the tpg / tpg-beta repos if there's interest.
Fixes hashicorp/terraform-provider-google#20134
Release Note Template for Downstream PRs (will be copied)
I made a first pass at some unit tests locally (this file currently has 0 coverage), but a bit stuck on the best way to mock the runner and whether or not to use
mock_runner_test.go
.Looking at downstream, I do see that there are a handful of cases where there's a note even though release notes is
none
, e.g., (in9889.txt
):It would probably be a little harder, but would be possible to retain those if we did want to send them downstream, though I think the changelog is only used for release notes anyway?