-
Notifications
You must be signed in to change notification settings - Fork 52
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
docs: Update README.md #541
Conversation
Adding an alternative option for installing slsa-verifier if you do not rely on additional tooling. The benefit of this option is improved readability. Signed-off-by: Drew Roen <102626803+drewroengoogle@users.noreply.github.com>
Alternatively, if your project does not rely on additional tools and only uses slsa-verifier, you can instead run the following commands: | ||
```bash | ||
$ cd tooling | ||
$ go install github.com/slsa-framework/slsa-verifier/v2/cli/slsa-verifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! We do already have a note on how to install at the beginning of this section, around line 126. Maybe it's hard to pick out and should get it's own section?
We'd also like folks to install a release rather than from HEAD.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think he means that you can install this way with the go.mod, instead of using grep _ ...
command.
The install will pick up the version defined in the go.mod. This is more readable if there's a single tool, ie just the slsa-verifier to install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking a look! I was trying to convey that you can replace the grep _ tooling_test.go ...
line with go install github.com/slsa-framework/slsa-verifier ...
in your CI and still get the release version that is set in go.mod
. I figured this might be a nice alternative for increased readability of what is happening in your CI if you don't have other dependencies you use, but let me know if there's a better way to phrase it. If you think it is too much of a duplicate of the installation option above, no worries :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add a section:
Option 1: Compile locally
Option 2: Compile in CI
...
wdut?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think he means that you can install this way with the go.mod, instead of using
grep _ ...
command.
Ah, I see. Though I don't think you need to be in the tooling directory to do that if your go.mod is in your project root so I think you could omit the cd tooling
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example we give separates the tooling.go. I think it's a feature so that we don't add dependencies to the main project. At least that's how I view it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should add a section:
Option 1: Compile locally Option 2: Compile in CI ...
wdut?
I can add it in this PR if it makes sense, let me know what you all think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ianlewis any more though on this? Or shall we merge as-in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'm ok with it.
Alternatively, if your project does not rely on additional tools and only uses slsa-verifier, you can instead run the following commands: | ||
```bash | ||
$ cd tooling | ||
$ go install github.com/slsa-framework/slsa-verifier/v2/cli/slsa-verifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I'm ok with it.
Adding an alternative option for installing slsa-verifier if you do not rely on additional tooling. The benefit of this option is improved readability.