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

1828: RFR emails still be sent when PR is in draft state #1484

Closed
wants to merge 2 commits into from

Conversation

lgxbslgx
Copy link
Member

@lgxbslgx lgxbslgx commented Mar 15, 2023

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in PR-1469. Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace

Issue

  • SKARA-1828: RFR emails still be sent when PR is in draft state

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/skara pull/1484/head:pull/1484
$ git checkout pull/1484

Update a local copy of the PR:
$ git checkout pull/1484
$ git pull https://git.openjdk.org/skara pull/1484/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1484

View PR using the GUI difftool:
$ git pr show -t 1484

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/skara/pull/1484.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Mar 15, 2023

👋 Welcome back gli! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

@openjdk openjdk bot changed the title SKARA-1828 1828: RFR emails still be sent when PR is in draft state Mar 15, 2023
@openjdk openjdk bot added the rfr label Mar 15, 2023
@mlbridge
Copy link

mlbridge bot commented Mar 15, 2023

Webrevs

Copy link
Member

@erikj79 erikj79 left a comment

Choose a reason for hiding this comment

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

I think this is fine. Perfect synchronization on commit emails isn't as important as for comments.

@openjdk
Copy link

openjdk bot commented Mar 15, 2023

@lgxbslgx This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

🔍 One or more changes in this pull request modifies files in areas of the source code that often require two reviewers. Please consider if this is the case for this pull request, and if so, await a second reviewer to approve this pull request before you integrate it.

🌎 Applicable reviewers for one or more changes in this pull request are spread across multiple different time zones. Please consider waiting with integrating this pull request until it has been out for review for at least 24 hours to give all reviewers a chance to review the pull request.

After integration, the commit message for the final commit will be:

1828: RFR emails still be sent when PR is in draft state

Reviewed-by: erikj

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

@openjdk openjdk bot added the ready label Mar 15, 2023
@mlbridge
Copy link

mlbridge bot commented Mar 15, 2023

Mailing list message from Guoxiong Li on skara-dev:

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in [PR-1469](https://github.com//pull/1469#issuecomment-1425916242). Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong

-------------

Commit messages:
- SKARA-1828

Changes: https://git.openjdk.org/skara/pull/1484/files
Webrev: https://webrevs.openjdk.org/?repo=skara&pr=1484&range=00
Issue: https://bugs.openjdk.org/browse/SKARA-1828
Stats: 31 lines in 2 files changed: 30 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/skara/pull/1484.diff
Fetch: git fetch https://git.openjdk.org/skara pull/1484/head:pull/1484

PR: https://git.openjdk.org/skara/pull/1484

@mlbridge
Copy link

mlbridge bot commented Mar 15, 2023

Mailing list message from Erik Joelsson on skara-dev:

On Wed, 15 Mar 2023 13:50:16 GMT, Guoxiong Li <gli at openjdk.org> wrote:

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in [PR-1469](https://github.com//pull/1469#issuecomment-1425916242). Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong

I think this is fine. Perfect synchronization on commit emails isn't as important as for comments.

-------------

Marked as reviewed by erikj (Lead).

PR: https://git.openjdk.org/skara/pull/1484

@mlbridge
Copy link

mlbridge bot commented Mar 15, 2023

Mailing list message from Guoxiong Li on skara-dev:

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in [PR-1469](https://github.com//pull/1469#issuecomment-1425916242). Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong

Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:

- Merge branch 'master' into SKARA-1828
- SKARA-1828

-------------

Changes:
- all: https://git.openjdk.org/skara/pull/1484/files
- new: https://git.openjdk.org/skara/pull/1484/files/d223a7d2..c8ae07c

Webrevs:
- full: https://webrevs.openjdk.org/?repo=skara&pr=1484&range=01
- incr: https://webrevs.openjdk.org/?repo=skara&pr=1484&range=00-01

Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod
Patch: https://git.openjdk.org/skara/pull/1484.diff
Fetch: git fetch https://git.openjdk.org/skara pull/1484/head:pull/1484

PR: https://git.openjdk.org/skara/pull/1484

@zhaosongzs
Copy link
Member

zhaosongzs commented Mar 15, 2023

It's strange that mlbridge bot bridged emails that generated by itself. Filed SKARA-1843

@lgxbslgx
Copy link
Member Author

/integrate

@openjdk
Copy link

openjdk bot commented Mar 16, 2023

Going to push as commit add1d7f.
Since your change was applied there have been 2 commits pushed to the master branch:

  • 0c82310: 1813: URLs printed by Skara should have ".git" at the end of the repo name
  • aa6a5a8: 1824: /reviewers N should remove ready status for clean backports

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated label Mar 16, 2023
@openjdk openjdk bot closed this Mar 16, 2023
@openjdk
Copy link

openjdk bot commented Mar 16, 2023

@lgxbslgx Pushed as commit add1d7f.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@lgxbslgx lgxbslgx deleted the SKARA-1828 branch March 16, 2023 04:14
@mlbridge
Copy link

mlbridge bot commented Mar 16, 2023

Mailing list message from Guoxiong Li on skara-dev:

Test normal bridged message.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/skara-dev/attachments/20230316/6fe6478d/attachment.htm>

@mlbridge
Copy link

mlbridge bot commented Mar 17, 2023

Mailing list message from Zhao Song on skara-dev:

On Wed, 15 Mar 2023 14:36:19 GMT, Guoxiong Li <gli at openjdk.org> wrote:

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in [PR-1469](https://github.com//pull/1469#issuecomment-1425916242). Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong

Guoxiong Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision:

- Merge branch 'master' into SKARA-1828
- SKARA-1828

It's strange that mlbridge bot bridged emails that generated by itself. Filed [SKARA-1843](https://bugs.openjdk.org/browse/SKARA-1843)

-------------

PR Comment: https://git.openjdk.org/skara/pull/1484#issuecomment-1470513955

@mlbridge
Copy link

mlbridge bot commented Mar 17, 2023

Mailing list message from Guoxiong Li on skara-dev:

On Wed, 15 Mar 2023 13:50:16 GMT, Guoxiong Li <gli at openjdk.org> wrote:

Hi all,

This patch blocks the RFR email when new commits are pushed to a draft PR. And the test case is added. too.

But it doesn't solve the synchronization problem that Erik mentioned in [PR-1469](https://github.com//pull/1469#issuecomment-1425916242). Because the bot only generates one email one time regradless of the number of the commits.

Thanks for the review.

Best Regards,
-- Guoxiong

This pull request has now been integrated.

Changeset: add1d7f
Author: Guoxiong Li <gli at openjdk.org>
URL: https://git.openjdk.org/skara/commit/add1d7fc686a5dd61c77711fd13b313b74e0f3b1
Stats: 31 lines in 2 files changed: 30 ins; 0 del; 1 mod

1828: RFR emails still be sent when PR is in draft state

Reviewed-by: erikj

-------------

PR: https://git.openjdk.org/skara/pull/1484

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants