prestodb / presto

The official home of the Presto distributed SQL query engine for big data
http://prestodb.io
Apache License 2.0
15.91k stars 5.33k forks source link

Add PR number to the release notes entry in a new PR #22678

Open steveburnett opened 4 months ago

steveburnett commented 4 months ago

Automatically add the number of the pull request to the release note entry example in the pull request.

Expected Behavior or Use Case

When a pull request is created, replace 12345 in https://github.com/prestodb/presto/blob/master/pull_request_template.md with the number of the newly generated pull request. See the screenshot in Example Screenshots.

Presto Component, Service, or Connector

The pull request generation process.

Possible Implementation

?

Example Screenshots (if appropriate):

When a PR is generated, the PR generation process should replace 12345 in this screenshot with the number of the new PR.

Screenshot 2024-05-03 at 4 43 57 PM

Context

Including the PR number in a release note entry in the Presto Release Notes is valuable to help readers of the release notes quickly find the PR a release note entry is sourced from. For an example, see the notes for Release 0.286.

Manually editing the individual release note entries during the release notes process to include the PR that the entry is sourced from takes significant work and time, and can lead to errors.

22665 addressed this by asking the creator of each PR to manually add the new PR number in the release note entry for their PR, and by improving the PR template with a placeholder formatted value for the release note to help the PR creator correctly format the PR number in their release note entry.

@rschlussel suggested this process could be improved if the new PR number could automatically be added to the new PR's release note entry example, and I offered to write the idea up in an issue.

ethanyzhang commented 4 months ago

Hi @ZacBlanco , this was an issue Tim shared with me. Do you think the engineers working with you on OSS can help with this? CC @tdcmeehan

ZacBlanco commented 3 months ago

I will see if I can get one of them to do it

ZacBlanco commented 3 months ago

@steveburnett do you know if there is a tool that we use which compiles the release notes from all PRs into a list for the release?

steveburnett commented 3 months ago

@steveburnett do you know if there is a tool that we use which compiles the release notes from all PRs into a list for the release?

That would be a question for @wanglinsong and I think would be related to how he creates the PR for each release's release notes, for example #22647.

Having answered your question (admittedly by saying "ask someone else"), I hoped to address this problem when the individual PR is first opened.

I was hoping that this would improve the release note entry in a given PR, so that it is ready to be gathered using the process @wanglinsong uses to gather the release notes and create the content of the release's release notes. Does that make sense?

ZacBlanco commented 3 months ago

It makes sense, but currently GitHub's PR templates don't support dynamic content, so I see two solutions: (1) is set up a bot that scans PRs and automatically edits PRs to add the PR number, or (2) just update the script which gathers the release notes and add them during that process. I think the 2nd solution is simpler.

steveburnett commented 3 months ago

"currently GitHub's PR templates don't support dynamic content in PR templates" - I didn't know that, thanks! Given that fact, yes I agree with you.

rschlussel commented 3 months ago

There is a tool for generating the release notes and it lives in the presto-release-tools repo. See https://github.com/prestodb/presto-release-tools/blob/7ac6685673a1741d98dd30b8756c6474ac818c36/presto-release-tools/src/main/java/com/facebook/presto/release/tasks/GenerateReleaseNotesTask.java#L77

ZacBlanco commented 3 months ago

Thanks for the pointer @rschlussel!

@yzhang1991 I decided I'd rather have the new OSS contributors work on the Presto codebase directly. I opened a PR to add this on the presto-release-tools repo. This was a quick fix anyways. I'd appreciate if someone could review this PR: https://github.com/prestodb/presto-release-tools/pull/27

steveburnett commented 3 months ago

If this is solved, then I should probably undo the doc changes in #22665. Yes?

tdcmeehan commented 3 months ago

Almost, we'll need to cut a new release for the release tool first.

ZacBlanco commented 3 months ago

Re-opening this issue until the release is cut

steveburnett commented 3 months ago

Has a new release for the release tool been cut?