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

Update README instructions #143

Merged
merged 11 commits into from
Sep 22, 2023

Conversation

sjspielman
Copy link
Member

Closes #112

This PR updates the README with instructions for setting up. While working on this, I also thought of one more setup issue that we should have - customizing participant info - and added to the GHA.

I ended up removing the issues checklist from the README in favor of just telling folks to run the github action, which I think is much simpler. I also left a section in the README to circle back to (will open an issue for it) about additional external instructions. One reason to circle back to this is that, honestly, I can't think of any external-specific instructions... can you? Do we even need that subsection?

Any feedback on phrasing, organization, preference to restore that checklist, let me know!

Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

Overall this looks good, but a few comments

README.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 12 to 13
We recommend setting the new repository to public.
(When you use GitHub pages on a private repository, those pages are publicly accessible anyway.)
Copy link
Member

Choose a reason for hiding this comment

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

So, this is tricky: for outside users without paid plans, the repo must be public to use Pages. I don't know if we want to get into it more than we did before. (But also, capitalize "Pages"?)

Copy link
Member Author

Choose a reason for hiding this comment

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

First I have a typo there - should be "setting the new repo to private."

That said, I don't think what you wrote is true?

https://docs.github.com/en/pages/getting-started-with-github-pages/about-github-pages

Warning: GitHub Pages sites are publicly available on the internet, even if the repository for the site is private. If you have sensitive data in your site's repository, you may want to remove the data before publishing. For more information, see "About repositories."

Copy link
Member

Choose a reason for hiding this comment

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

Yes but also (from the green box):

GitHub Pages is available in public repositories with GitHub Free and GitHub Free for organizations, and in public and private repositories with GitHub Pro, GitHub Team, GitHub Enterprise Cloud, and GitHub Enterprise Server.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see... yeah..

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added more detail here, let me know what you think!

README.md Outdated
To run this action, navigate to the "Actions" tab.
On the left, you will see all available workflows.
Click the workflow named `Manually trigger issue creation for standard set up`, and then click the "Run workflow" dropdown button.
You will need to provide one required input, the `training-modules` repository release tag you would like to associate with the workshop.
Copy link
Member

Choose a reason for hiding this comment

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

Can this be updated later? If so, we should say that here, and probably mention the default. Or if not, we might say whether this release tag needs to exist, or if it is the one we will assign?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, looks like the only time this tag is used is for the title of an issue!

# Issue for creating a training-specific release of training-modules
- name: Create issue for training-specific release
uses: peter-evans/create-issue-from-file@v4.0.0
with:
title: Create ${{ github.event.inputs.tag }} release of training-modules
content-filepath: ./setup-issue-templates/workshop-release.md
labels: automated training issue

I think it's worth just getting rid of this input since there's enough info in the issue template itself to create the release:
https://github.com/AlexsLemonade/training-specific-template/blob/main/setup-issue-templates/workshop-release.md

Copy link
Member

Choose a reason for hiding this comment

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

I agree with this decision, especially since the issue itself has detail on the expected format that the action prompt does not!

setup-issue-templates/user-facing-readme.md Outdated Show resolved Hide resolved
Copy link
Member

@jashapiro jashapiro left a comment

Choose a reason for hiding this comment

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

LGTM, with one tiny wording suggestion.

README.md Outdated Show resolved Hide resolved
Co-authored-by: Joshua Shapiro <josh.shapiro@ccdatalab.org>
@sjspielman sjspielman merged commit 8274a1b into template-refresh Sep 22, 2023
1 check passed
@sjspielman sjspielman deleted the sjspielman/112-readme-instructions branch September 22, 2023 13:22
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