Improve contribution documentation #572

Stephan202 · 2023-04-08T13:23:11Z

Suggested commit message:

Improve contribution documentation (#572)

- Introduce a `./run-full-build.sh` script.
- Explicitly mention that users should run this script before opening a pull
  request.
- Emphasize that many build warnings can be resolved automatically.
- Introduce a `SECURITY.md` file as suggested by GitHub.

Context of this PR: I've registered Error Prone Support with the OpenSSF Best Practices Badge Program, and these changes increase our compliance. (Other PRs to improve compliance may follow.)

github-actions · 2023-04-08T13:24:55Z

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

japborst · 2023-04-09T07:16:17Z

SECURITY.md

+will take such reports seriously and work with you to resolve the issue in a
+timely manner.
+
+[security-advisories]: https://github.com/PicnicSupermarket/error-prone-support/security/advisories


Did not know this existed. Nice!

japborst · 2023-04-09T07:17:55Z

CONTRIBUTING.md

+- Make sure that the `mvn clean install` build fully passes, ideally before
+  opening a pull request. See the [development


Ideally? Or should we make this more mandatory

Suggested change

- Make sure that the `mvn clean install` build fully passes, ideally before

opening a pull request. See the [development

- Before opening a Pull Request, make sure that the `mvn clean install`

build fully passes. See the [development

I was actually thinking about this last night, and realized that if people get stuck, this requirement might discourage them from contributing at all. I proposed a new sentence; PTAL :)

sonarqubecloud · 2023-04-09T10:08:10Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

github-actions · 2023-04-09T10:09:54Z

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

japborst · 2023-04-10T07:29:37Z

FYI there's a typo on https://bestpractices.coreinfrastructure.org/en/projects/7199. s/Errorc/Error

Stephan202 · 2023-04-10T07:49:30Z

🤦 Fixed!

rickie

Added a commit. PTAL :). Overall, LGTM!

rickie · 2023-04-10T13:31:50Z

SECURITY.md

+timely manner.
+
+[security-advisories]: https://github.com/PicnicSupermarket/error-prone-support/security/advisories
+[semantic-versioning]: https://semver.org/


Suggested change

[semantic-versioning]: https://semver.org/

[semantic-versioning]: https://semver.org

rickie · 2023-04-10T13:32:02Z

SECURITY.md

+## Reporting a vulnerability
+
+To report a vulnerability, please visit the [security
+advisories][security-advisories] page an click _Report a vulnerability_. We


Suggested change

advisories][security-advisories] page an click _Report a vulnerability_. We

advisories][security-advisories] page and click _Report a vulnerability_. We

rickie · 2023-04-11T06:40:54Z

CONTRIBUTING.md

+  That said, if you feel that the build fails for invalid or debatable reasons,
+  or if you're unsure how to best resolve an issue, don't let that discourage
+  you from opening a PR with a failing build; we can have a look at the issue
+  together.


While we are at it, I would like to propose to add running the selfcheck here. PTAL.

github-actions · 2023-04-11T06:43:24Z

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie · 2023-04-11T06:45:05Z

Tweaked suggested commit message as well.

Stephan202

I think we can do better here, e.g. by checking in the build-all.sh script I use locally. But likely we should more generally refactor this information so that it's not spread across two files. That seems like a larger task, though...

Stephan202 · 2023-04-11T07:05:46Z

CONTRIBUTING.md

+- Make sure that there are no violations of the newly introduced checks in the
+  codebase itself by running the [self check][error-prone-support-self-check].
+  See the [development instructions][error-prone-support-developing] for extra
+  context.


This mostly repeats the text above. The idea behind referencing the error-prone-support-developing was that we don't need to repeat the self check description.

Stephan202 · 2023-04-11T07:06:28Z

CONTRIBUTING.md

 [error-prone-support-issues]: https://github.com/PicnicSupermarket/error-prone-support/issues
 [error-prone-support-mutation-tests]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/run-mutation-tests.sh
 [error-prone-support-pulls]: https://github.com/PicnicSupermarket/error-prone-support/pulls
+[error-prone-support-self-check]: https://github.com/PicnicSupermarket/error-prone-support/blob/master/apply-error-prone-suggestions.sh


This is the auto-fix script, not a full self check. (The details are subtle, which is why I was thinking to only reference the other section.)

rickie · 2023-04-11T08:15:38Z

I'm not sure everyone reads till the end in the error-prone-support-developing section as the explanation is below "some other commands one may find relevant". We could also reword that to say something along the lines of "important/helpful to run before opening a PR". That would highlight the actual importance IMO.

Stephan202 · 2023-04-11T09:13:56Z

K, I'll get back to this later for a more holistic approach, then.

Stephan202 · 2023-04-12T06:44:56Z

Added a commit in which I reshuffled and emphasized a few things. Also added a script to run the "full build".

github-actions · 2023-04-12T06:46:34Z

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie

💯 good stuff! Really nice improvements :).

rickie · 2023-04-13T06:09:18Z

Will rebase and merge this.

- Explicitly mention that users should run `mvn clean install` before opening a pull request. - Introduce a `SECURITY.md` file as suggested by GitHub.

github-actions · 2023-04-13T06:33:44Z

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

rickie · 2023-04-13T06:45:06Z

Tweaked suggested commit message PTAL.

Stephan202 · 2023-04-13T06:47:28Z

Good one; made some changes :)

rickie · 2023-04-13T06:49:58Z

Okay I indeed should've done better here 😅. Thanks! Will merge.

rickie · 2023-04-13T06:50:42Z

(Dropped one tiny word, something that is introduced is always new right 😉, and the last entry doesn't have "new").

Stephan202 added this to the 0.10.0 milestone Apr 8, 2023

Stephan202 requested review from japborst and rickie April 8, 2023 13:23

japborst reviewed Apr 9, 2023

View reviewed changes

japborst approved these changes Apr 10, 2023

View reviewed changes

rickie reviewed Apr 11, 2023

View reviewed changes

Stephan202 commented Apr 11, 2023

View reviewed changes

rickie added the documentation A documentation update label Apr 11, 2023

rickie approved these changes Apr 13, 2023

View reviewed changes

Stephan202 and others added 4 commits April 13, 2023 08:32

Improve contribution documentation

79d8c40

- Explicitly mention that users should run `mvn clean install` before opening a pull request. - Introduce a `SECURITY.md` file as suggested by GitHub.

Don't discourage people

a6c2227

Suggestion for the selfcheck and typos

40bafeb

Proposal

f00ad89

rickie force-pushed the sschroevers/doc-improvements branch from 21dd324 to f00ad89 Compare April 13, 2023 06:32

rickie merged commit 977019c into master Apr 13, 2023

rickie deleted the sschroevers/doc-improvements branch April 13, 2023 07:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve contribution documentation #572

Improve contribution documentation #572

Stephan202 commented Apr 8, 2023 •

edited by rickie

Loading

github-actions bot commented Apr 8, 2023

japborst Apr 9, 2023

japborst Apr 9, 2023

Stephan202 Apr 9, 2023

sonarqubecloud bot commented Apr 9, 2023

github-actions bot commented Apr 9, 2023

japborst commented Apr 10, 2023

Stephan202 commented Apr 10, 2023

rickie left a comment

rickie Apr 10, 2023

rickie Apr 10, 2023

rickie Apr 11, 2023

github-actions bot commented Apr 11, 2023

rickie commented Apr 11, 2023

Stephan202 left a comment

Stephan202 Apr 11, 2023

Stephan202 Apr 11, 2023

rickie commented Apr 11, 2023

Stephan202 commented Apr 11, 2023

Stephan202 commented Apr 12, 2023

github-actions bot commented Apr 12, 2023

rickie left a comment

rickie commented Apr 13, 2023

github-actions bot commented Apr 13, 2023

rickie commented Apr 13, 2023

Stephan202 commented Apr 13, 2023

rickie commented Apr 13, 2023

rickie commented Apr 13, 2023

		- Make sure that the `mvn clean install` build fully passes, ideally before
		opening a pull request. See the [development

	[semantic-versioning]: https://semver.org/
	[semantic-versioning]: https://semver.org

	advisories][security-advisories] page an click _Report a vulnerability_. We
	advisories][security-advisories] page and click _Report a vulnerability_. We

Improve contribution documentation #572

Improve contribution documentation #572

Conversation

Stephan202 commented Apr 8, 2023 • edited by rickie Loading

github-actions bot commented Apr 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 9, 2023

github-actions bot commented Apr 9, 2023

japborst commented Apr 10, 2023

Stephan202 commented Apr 10, 2023

rickie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 11, 2023

rickie commented Apr 11, 2023

Stephan202 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rickie commented Apr 11, 2023

Stephan202 commented Apr 11, 2023

Stephan202 commented Apr 12, 2023

github-actions bot commented Apr 12, 2023

rickie left a comment

Choose a reason for hiding this comment

rickie commented Apr 13, 2023

github-actions bot commented Apr 13, 2023

rickie commented Apr 13, 2023

Stephan202 commented Apr 13, 2023

rickie commented Apr 13, 2023

rickie commented Apr 13, 2023

Stephan202 commented Apr 8, 2023 •

edited by rickie

Loading