skara
skara copied to clipboard
2170: Warn on trailing period in PR titles
This patch is trying to add a new jcheck called "issuestitle". This jcheck would check if the issue's title has a trailing period or leading lowercase letter. And we would like to configure "issuestitle" jcheck as warning in the repos since in some cases, trailing periods or leading lowercase letter are valid.
When implementing this, I found that although we can configure a jcheck as warning in the jcheck configuration file(.jcheck/conf), we fail to differentiate between jchecks as error and jchecks as warning. When jchecks fail as warnings, they cause the local jcheck to exit with a status code of 1. Also, they will be integration blockers when Skara bots evaluate pull requests. Therefore, in this patch, I also make Skara CLI and Skara bots be able to handle jcheck warnings properly.
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
Issues
- SKARA-2170: Warn on trailing period in PR titles (Enhancement - P4)
- SKARA-2248: Warn on leading lowercase letter in PR titles (Enhancement - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/skara.git pull/1638/head:pull/1638
$ git checkout pull/1638
Update a local copy of the PR:
$ git checkout pull/1638
$ git pull https://git.openjdk.org/skara.git pull/1638/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 1638
View PR using the GUI difftool:
$ git pr show -t 1638
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/skara/pull/1638.diff
Webrev
:wave: Welcome back zsong! 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.
@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:
2170: Warn on trailing period in PR titles
2248: Warn on leading lowercase letter in PR titles
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 10 new commits pushed to the master
branch:
- 53f65c291fb28d2b5f9c6244d4d7f9319500587f: 2249: serviceability mailing list should be notified of changes to heapdumper and diagnostic files
- 695dbc6b9603b5c1b4237ce24d4aacf9d1461d1d: 2247: When pr's headHash is missing, stop further processing
- d8a8a24d35b1ee461e42364d8d6c2a1215a8ca8f: 2246: Check if the pull request is still open before issuing "integrate" automatically
- 7e810cac854f2f55cc7fb85f76b4ebc6cabaf2c6: 2234: /reviewers N should remove ready status for merge pull requests
- 052f7e0132a18f369164ca5c613df3be93eb80d4: 2226: Make it possible to avoid "forward backports"
- 2372cbbcc5d5b66715b8e0469b6ed798404435c8: 2245: Don't check security in JiraIssueTrackerFactory
- a13dcde9bc5f9c3575a6839957ee483ac00d7cc3: 2244: SecurityLevel in JiraHost is useless
- ac3c81f05fbe2d7ef5b690ca0819a3e1cb96340e: 2242: Creating pull request on github assumes source is personal fork of user
- 1dfc1aea12e7f334af142bbddc1f5e9db61ce38a: 2233: TestHost does not set assignees correctly for backports
- c69015f7fc0b63d6c121ebbd743ca39e15ab224e: 2239: Stop logging "issue: bad whitespace"
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.
➡️ To integrate this PR with the above commit message to the master
branch, type /integrate in a new comment.
I looked at the code to try to answer my own question, but could not find out an easy answer, so I'll say it out loud instead:
How do you make it so that this is a warning only, and not a blocking error?
I looked at the code to try to answer my own question, but could not find out an easy answer, so I'll say it out loud instead:
How do you make it so that this is a warning only, and not a blocking error?
Hi Magnus, I haven't finished this patch... So it's still in the draft state. I will finish it today.
Webrevs
- 03: Full - Incremental (b02f549e)
- 02: Full - Incremental (584bcbfe)
- 01: Full - Incremental (2a4e6ee0)
- 00: Full (4c6364b7)
/issue add SKARA-2248
@zhaosongzs
Adding additional issue to issue list: 2248: Warn on leading lowercase letter in PR titles
.
This PR is ready for review. Could someone please review it? ping...
This PR is ready for review. Could someone please review it? ping...
Sorry, I saw your comment about it not being ready so assumed you would say something when it was. Noticed the draft state change now.
Thanks for the review! /integrate
Going to push as commit 2edf2280cbc800a7e578e134d68fe9b33f8e1bd7.
Since your change was applied there have been 10 commits pushed to the master
branch:
- 53f65c291fb28d2b5f9c6244d4d7f9319500587f: 2249: serviceability mailing list should be notified of changes to heapdumper and diagnostic files
- 695dbc6b9603b5c1b4237ce24d4aacf9d1461d1d: 2247: When pr's headHash is missing, stop further processing
- d8a8a24d35b1ee461e42364d8d6c2a1215a8ca8f: 2246: Check if the pull request is still open before issuing "integrate" automatically
- 7e810cac854f2f55cc7fb85f76b4ebc6cabaf2c6: 2234: /reviewers N should remove ready status for merge pull requests
- 052f7e0132a18f369164ca5c613df3be93eb80d4: 2226: Make it possible to avoid "forward backports"
- 2372cbbcc5d5b66715b8e0469b6ed798404435c8: 2245: Don't check security in JiraIssueTrackerFactory
- a13dcde9bc5f9c3575a6839957ee483ac00d7cc3: 2244: SecurityLevel in JiraHost is useless
- ac3c81f05fbe2d7ef5b690ca0819a3e1cb96340e: 2242: Creating pull request on github assumes source is personal fork of user
- 1dfc1aea12e7f334af142bbddc1f5e9db61ce38a: 2233: TestHost does not set assignees correctly for backports
- c69015f7fc0b63d6c121ebbd743ca39e15ab224e: 2239: Stop logging "issue: bad whitespace"
Your commit was automatically rebased without conflicts.
@zhaosongzs Pushed as commit 2edf2280cbc800a7e578e134d68fe9b33f8e1bd7.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.