-
Notifications
You must be signed in to change notification settings - Fork 6
Stop filtering out PRs that were also backported to previous versions #57
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
Stop filtering out PRs that were also backported to previous versions #57
Conversation
|
@Ikuni17 could you take a look? 🙏 |
bmorelli25
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome sauce! A couple comments. Note: I did not test this PR.
Can you update the documentation in https://github.com/elastic/kibana-release-notes/blob/d1ab661cd05ccabaa5a5607bef137c1198d580ba/src/pages/release-notes/wizard.tsx#L326-327
Actually I've removed this wizard step as it is not useful with the new expected behavior in my latest commit |
|
My concern would be if this change makes the Kibana release notes have different behaviour than the other products' docs. [edit] It looks like Elasticsearch is following this same pattern, e.g. I see that elastic/elasticsearch#137222 appears in the 9.1.7 and 9.2.1 sections of https://www.elastic.co/docs/release-notes/elasticsearch, as well as in https://www.elastic.co/guide/en/elasticsearch/reference/8.19/release-notes-8.19.7.html I wondered how this overlapped with the exclusion of PRs that have
... but after a quick test locally with a PR like elastic/kibana#242691 it does show up in the 9.1.8 release notes (but its backport PR elastic/kibana#242721 does not), which I think is working as hoped. |
|
@lcawl it's been consistently requested for multiple teams. I think ES does it, and even for Kibana/solutions we've been doing it as well (or trying to as it appears we missed some PRs) for some months.
This is still the desired behavior - as PRs with the |
Ikuni17
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for doing this, nice work! I was always confused about picking those extra versions and wanted to fix this.
I pushed a few small commits to refactor and add tests
abec88e...cc2052f
I ran everything locally and LGTM. If you could quickly rerun my changes locally incase I missed something and then merge.
|
Admin merging, CI passed here: https://github.com/florent-leborgne/kibana-release-notes/actions/runs/19450049965 |
This PR:
Tested locally to check that the updated behaviour works as expected ✅:
Fixes: #50
Fixes the behavior raised in https://elastic.slack.com/archives/C0JF80CJZ/p1763136763466579