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

Improve documentation #125

Merged
merged 4 commits into from Oct 4, 2021
Merged

Improve documentation #125

merged 4 commits into from Oct 4, 2021

Conversation

luhring
Copy link
Contributor

@luhring luhring commented Oct 3, 2021

This PR improves scan-action's documentation in the following ways:

  1. Adds markdown badges to README.md
  2. Replaces CONTRIBUTING.rst with our standard CONTRIBUTING.md (and its new content)

Signed-off-by: Dan Luhring <dan.luhring@anchore.com>
Signed-off-by: Dan Luhring <dan.luhring@anchore.com>
@luhring luhring requested a review from a team October 3, 2021 14:53
@luhring luhring self-assigned this Oct 3, 2021
CONTRIBUTING.md Outdated
Comment on lines 86 to 97
This project has a `Makefile` which includes many helpers running both unit and integration tests. Although PRs will have automatic checks for these, it is useful to run them locally, ensuring they pass before submitting changes. Ensure you've bootstrapped once before running tests:

```text
$ make bootstrap
```

You only need to bootstrap once. After the bootstrap process, you can run the tests as many times as needed:

```text
$ make unit
$ make integration
```
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't quite right, should possibly just reference Developing tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch — yeah, this is wrong. That hyperlink doesn't quite work for me when I try clicking it, but I'll try to find the relevant section...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 2b0464e!

@@ -1,6 +1,9 @@
[![Test Status][test-img]][test]
# GitHub Action for Vulnerability Scanning
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought I saw elsewhere we were going for sentence case? Or possibly since this is the title, we have an exception for title case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kzantow I don't have a strong opinion! Since this is the title of the README itself, I chose this case, but I'm happy to change it! Let me know. (What was there before was a capital "S" in "scanning", but a lowercase "v" in "vulnerability")

Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

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

Good after the note about the test section in CONTRIBUTING.md

Signed-off-by: Dan Luhring <dan.luhring@anchore.com>
CONTRIBUTING.md Outdated

## Test your changes

Ensure that your changes have passed the test suite. For more information on working with this project's tests, see https://github.com/anchore/scan-action/tree/main/tests#developing-tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just an FYI... I'm pretty sure if you end up viewing this file, you'll be at a URL like: https://github.com/anchore/scan-action/blob/main/CONTRIBUTING.md and the test readme is then https://github.com/anchore/scan-action/blob/main/tests/README.md, so the relative URL of tests/README.md#title should work, although it won't in PRs because you're at https://github.com/anchore/scan-action/pull/125/files

Copy link
Contributor

Choose a reason for hiding this comment

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

And if you go to View file:
image
I think those relative links work, too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kzantow Okay, this is good to know... do you see it as a good idea to change the kind of link here? (curious what the pros/cons are)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think relative links are best, so you could potentially review a PR with changes across them otherwise you're linking back to master. Not a strong opinion, but lean towards relative links when possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in 16c70ca — @kzantow How's that look?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll ask for forgiveness 😁

Signed-off-by: Dan Luhring <dan.luhring@anchore.com>
@luhring luhring merged commit 2965058 into main Oct 4, 2021
@luhring luhring deleted the improve-documentation branch October 4, 2021 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants