RFR: 8231601: Update CONTRIBUTING.md to clarify process for contributing features plus Skara changes

Nir Lisker nlisker at openjdk.java.net
Mon Sep 14 21:16:39 UTC 2020


On Sat, 12 Sep 2020 17:16:38 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

> This PR updates the `CONTRIBUTING.md` guide to address the following:
> 
> 1. Clarify the process for adding new features / API changes, specifically that they must be discussed on the mailing
> list as the first step. 2. Add a link to the mailing list in the section regarding contributing bug fixes.
> 3. Remove the text about cross-linking the PR and JBS issue, and add a note that the Skara tooling takes care of this
> 4. Remove the section about manually resolving the JBS issue, and add a note that the Skara bot automatically does this
> when the PR is integrated. 5. Suggest the use of the "/reviewers 2" and "/csr" commands when appropriate
> 6. Update the note regarding which JDK(s) to use.

Additional comments:

1. "Use Unix-style (LF) line endings not DOS-style (CRLF)" needs a comma before "not".
2. "Line width is no more than 120 characters" I remember that it was 130 or 135 somewhere.
3. "Wildcard imports (import foo.bar.baz.*) are forbidden" Junit imports use them extensively.
4. `./gradlew all test` will cause failure on webkit tests if it was not built.

CONTRIBUTING.md line 18:

> 16: ----------------
> 17:
> 18: All new feature requests, including any API changes, need prior discussion on the
> [openjfx-dev](mailto:openjfx-dev at openjdk.java.net) mailing list, even if there is already an open

I think that the mailing list link shouldn't be to a `mailto` protocol as these aren't always configured in the
browser, and in any case, since the intention is to have a discussion, the user should be advised to register to the
list and not send a one-time mail. So I suggest to redirect to
https://mail.openjdk.java.net/mailman/listinfo/openjfx-dev.

CONTRIBUTING.md line 19:

> 17:
> 18: All new feature requests, including any API changes, need prior discussion on the
> [openjfx-dev](mailto:openjfx-dev at openjdk.java.net) mailing list, even if there is already an open 19: [JBS
> issue](https://bugs.openjdk.java.net). See the [New features / API additions](#new-features--api-additions) section at
> the end of this guide for more information.

Also for the link under "Bug reports". I think that the issue list link should direct to a JavaFX search like
`https://bugs.openjdk.java.net/issues/?jql=component %3D javafx order by updated DESC` to make it easier to search.

CONTRIBUTING.md line 24:

> 22: -------------------------------------------
> 23:
> 24: If you have a bug fix or new feature that you would like to contribute to OpenJFX, please find or open an issue
> about it first. Talk about what you would like to do on the [openjfx-dev](mailto:openjfx-dev at openjdk.java.net) mailing
> list. It may be that somebody is already working on it, or that there are particular issues that you should know about
> before implementing the change. Feature requests, in particular, must be discussed ahead of time and will require
> significant effort on your part.

"please find or open an issue about it first"
Shouldn't a discussion happen before a feature is submitted in JBS? Or do we close as "won't fix" if the feature is
rejected?

CONTRIBUTING.md line 24:

> 22: -------------------------------------------
> 23:
> 24: If you have a bug fix or new feature that you would like to contribute to OpenJFX, please find or open an issue
> about it first. Talk about what you would like to do on the [openjfx-dev](mailto:openjfx-dev at openjdk.java.net) mailing
> list. It may be that somebody is already working on it, or that there are particular issues that you should know about
> before implementing the change. Feature requests, in particular, must be discussed ahead of time and will require
> significant effort on your part.

Same comment on the "openjfx-dev" link.

CONTRIBUTING.md line 88:

> 86:     of the PR title and check for whitespace errors. Once that passes,
> 87:     it will automatically send a Request For Review (RFR) email to the
> 88:     [openjfx-dev](mailto:openjfx-dev at openjdk.java.net) mailing list.

Same comment on the "openjfx-dev" link.

CONTRIBUTING.md line 96:

> 94:     TIP: prefix the pull request title with `WIP:` if you aren't yet
> 95:     ready for it to be reviewed. The Skara bot will not send an RFR
> 96:     email unless the title starts with a 7-digit bug ID.

I'm pretty sure it doesn't send RFR mails on draft issues regardless of the title. I create all my PRs as draft to make
sure everything is correct, and only then transition. I've never seen a premature RFR mail, though it could be a
natural delay of he bot.

CONTRIBUTING.md line 100:

> 98:     Please adhere to the general guideline that you should never force push
> 99:     to a publicly shared branch. Once you have opened your pull request, you
> 100:     should consider your branch publicly shared. Instead of force pushing

Minor: this whole paragraph is filled with git terminology like squash, rebase, force push... Maybe a link to the git
docs could help.

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

PR: https://git.openjdk.java.net/jfx/pull/303


More information about the openjfx-dev mailing list