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

Olivekl new install instructions #46

Merged
merged 31 commits into from
Jan 12, 2022

Conversation

olivekl
Copy link
Contributor

@olivekl olivekl commented Jan 6, 2022

New install instructions, including screenshots.

@laurentsimon
Copy link
Contributor

DRAFT, DO NOT MERGE

Draft of new installation instructions. Sharing for comments before I add screenshots and links.

Questions: -Is everything accurate and easy to follow?

I think we could get rid of it.

-Do we need to include the "manual" option at the bottom? Will anyone want to set up the action values themselves, or are they useful for any other reason?

I copied this from other actions' README. But other actions out there are not available under the Security tab, so that's why they need it.

@azeemshaikh38 wdut?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@azeemshaikh38
Copy link
Contributor

DRAFT, DO NOT MERGE
Draft of new installation instructions. Sharing for comments before I add screenshots and links.
Questions: -Is everything accurate and easy to follow?

I think we could get rid of it.

-Do we need to include the "manual" option at the bottom? Will anyone want to set up the action values themselves, or are they useful for any other reason?

I copied this from other actions' README. But other actions out there are not available under the Security tab, so that's why they need it.

@azeemshaikh38 wdut?

I think its ok to keep the manual part of the setup for more advanced users.

Address comments on draft PR
@olivekl
Copy link
Contributor Author

olivekl commented Jan 7, 2022

Thanks, @azeemshaikh38 . I addressed your comments. I'll add screenshots for the UI section if everything else looks good.

New folder for installation screenshot (delete placeholder.md after merging)
Screenshots for installation instructions
Remove placeholder file, no longer needed
Add screenshots to install instructions.
Change suggested name of secret to SCORECARD_READ_SECRET so users know it's read only
Add token permission image to images folder
Change SCORECARD_READ_SECRET back to _TOKEN 
Add screenshot of permissions
@olivekl olivekl marked this pull request as ready for review January 10, 2022 22:19
@laurentsimon laurentsimon mentioned this pull request Jan 10, 2022
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Address PR comments
Address PR comments
Add images for installation screenshots
README.md Outdated Show resolved Hide resolved
Update private repo support description
Fix typo
Update Publish Results and workflow example to account for private repo support
delete image to replace
delete image to replace
Add screenshots of confirming actions and remediation expansion
Try to fix line wrapping issue
Add spaces before images
Copy link
Contributor

@laurentsimon laurentsimon left a comment

Choose a reason for hiding this comment

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

Thank you!

@laurentsimon
Copy link
Contributor

We just need to rebase the PR.

@laurentsimon laurentsimon merged commit 13e780c into ossf:main Jan 12, 2022
@olivekl olivekl deleted the olivekl-new-install-instructions branch January 12, 2022 20:37
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

3 participants