skara
skara copied to clipboard
openjdk
1452: Can never notify on first commit
I changed the logic when there is no notify history.
Originally, on the first commit of a new repo, the bot would find no notify history, so it will make the first commit as an initial state and it will not make any notification.
Now, on the first commit of a new repo, the bot would find no notify history too, but it will make the initial git hash as the initial state of the repo, so it will treat the first commit as an 'update' operation and will make notifications.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
Issue
- SKARA-1452: Can never notify on first commit
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/skara pull/1385/head:pull/1385
$ git checkout pull/1385
Update a local copy of the PR:
$ git checkout pull/1385
$ git pull https://git.openjdk.org/skara pull/1385/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1385
View PR using the GUI difftool:
$ git pr show -t 1385
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/skara/pull/1385.diff
:wave: Welcome back zhaosongzs! 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.
![bridgekeeper[bot] avatar](https://avatars.githubusercontent.com/u/41768318?v=4 "Published by a pub.dev verified publisher"){kind=small}
Webrevs
- 08: Full - Incremental (3c6e4cfd)
- 07: Full - Incremental (36aeedbc)
- 06: Full - Incremental (5be09539)
- 05: Full - Incremental (feff1dbc)
- 04: Full - Incremental (4ee9a204)
- 03: Full - Incremental (c8dc670f)
- 02: Full - Incremental (dd9c63e6)
- 01: Full - Incremental (f5397b99)
- 00: Full (9c637fc3)
Solving this is more complex. I added more information to the bug.
Hi Erik,
I want to confirm whether I understand you correctly.
Case1: Completely new repo or repo with few commits In this situation, we should let the notify bot make notifications on the first commit and make notifications on every new commit.
Case2: Repo with a long history In this situation, we should let the notify bot not make notifications on the first commit and make notifications on every new commit.
And currently, My solution will let the notify bot make notifications on the first commit whether the repo is new or the repo has a long history, so I should fix it.
I want to confirm whether I understand you correctly.
Case1: Completely new repo or repo with few commits In this situation, we should let the notify bot make notifications on the first commit and make notifications on every new commit.
Case2: Repo with a long history In this situation, we should let the notify bot not make notifications on the first commit and make notifications on every new commit.
And currently, My solution will let the notify bot make notifications on the first commit whether the repo is new or the repo has a long history, so I should fix it.
I think you got it right, but I want to clarify a little bit more.
Case1: Notify on every commit, starting from the very first in each branch that should be notified on. Case2: Mark the currently existing HEAD in each branch as already notified and then continue notifying from there for any new commit that appears.
I would suggest a default threshold of 5 existing commits to trigger case1 (either define a constant for it or maybe introduce a configuration parameter).
@zhaosongzs 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.
:mag: 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.
After integration, the commit message for the final commit will be:
1452: Can never notify on first commit
Reviewed-by: erikj, gli
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 10 new commits pushed to the master branch:
- d6c5a1a84d8f80ccf6d39132080cb52982fab854: 1584: Require re-review of a PR if the target branch changes
- 55084b1bc6ff1d1d3261c19347a6eb3ec120104d: 1618: CSR bot failed to update PR when CSR was approved
- 68b4f58708ef4c113a81ce38455192a692f4c86b: 1626: 8-pool records not consumed by openjdk8uX fixversions
- 4b40646da7589e3dd88e34d00112bddc3dd1428b: 1606: PullRequestPoller always processes the last updated MR for GitLab
- b7b4d114a01cb703f35450ffe4820ce047fd235b: 1616: Improve error message when an author isn't found in GitLab
- 32a07238fde7773d64f0330e76d77f8d5d44f82a: 1603: Make labels handling consistent in all Issue implementations
- ae79322ae8dd6ce91e00d46f84139020e3b55f90: 1563: The method 'CheckRun#updateMergeReadyComment' shouldn't update the comment if the comment has not changed
- 522d894fded2cf99b88d7873de5b69faed161cd4: 1215: Clean label not updating correctly
- b061577e3ed45b464e73b865d8b73528b9166e07: 1612: Temp workaround GitLab returning 500 when modifying labels
- a6860bcba94d93a2be7429ad7a8766755a925581: 1364: Jira Issues should be resolved as "Fixed"
Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.
As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@erikj79, @lgxbslgx) but any other Committer may sponsor as well.
➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).
@zhaosongzs Your change (at version 3c6e4cfd32c4ddaee3425f11ada2bdc343a2fc3c) is now ready to be sponsored by a Committer.
Going to push as commit a9997ca4daff538c46f4d25f0c7048191b0b5e1d.
Since your change was applied there have been 10 commits pushed to the master branch:
- d6c5a1a84d8f80ccf6d39132080cb52982fab854: 1584: Require re-review of a PR if the target branch changes
- 55084b1bc6ff1d1d3261c19347a6eb3ec120104d: 1618: CSR bot failed to update PR when CSR was approved
- 68b4f58708ef4c113a81ce38455192a692f4c86b: 1626: 8-pool records not consumed by openjdk8uX fixversions
- 4b40646da7589e3dd88e34d00112bddc3dd1428b: 1606: PullRequestPoller always processes the last updated MR for GitLab
- b7b4d114a01cb703f35450ffe4820ce047fd235b: 1616: Improve error message when an author isn't found in GitLab
- 32a07238fde7773d64f0330e76d77f8d5d44f82a: 1603: Make labels handling consistent in all Issue implementations
- ae79322ae8dd6ce91e00d46f84139020e3b55f90: 1563: The method 'CheckRun#updateMergeReadyComment' shouldn't update the comment if the comment has not changed
- 522d894fded2cf99b88d7873de5b69faed161cd4: 1215: Clean label not updating correctly
- b061577e3ed45b464e73b865d8b73528b9166e07: 1612: Temp workaround GitLab returning 500 when modifying labels
- a6860bcba94d93a2be7429ad7a8766755a925581: 1364: Jira Issues should be resolved as "Fixed"
Your commit was automatically rebased without conflicts.
@lgxbslgx @zhaosongzs Pushed as commit a9997ca4daff538c46f4d25f0c7048191b0b5e1d.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.