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

Lg update pr instructions #763

Merged
merged 8 commits into from
Oct 30, 2024
Merged

Lg update pr instructions #763

merged 8 commits into from
Oct 30, 2024

Conversation

lgibson7
Copy link
Member

@lgibson7 lgibson7 commented Oct 29, 2024

I made some edits to the PR instructions.

Edits I didn't make but think we should clarify: What is meant by directory name in the saving.R file? Is that the same as the folder they created in their branch? Is the {dataset}.md file generated after running saving.R?

@lgibson7 lgibson7 requested a review from jonthegeek October 29, 2024 02:29
Copy link
Collaborator

@jonthegeek jonthegeek left a comment

Choose a reason for hiding this comment

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

Thanks! Hopefully this will help make it easier for people to contribute!

@jonthegeek
Copy link
Collaborator

I made some tweaks:

  • The {#whatever} tags aren't necessary in GitHub markdown, and showed up as text in the rendered document
  • The line breaks at periods don't display to the viewer, but they make it (much) easier to see changes in PRs, so I added them back in.
  • I updated curated/template/instructions.md to match these changes.
  • I clarified the folder instructions in saving.R. Good catch!
  • I clarified the source of {dataset}.md files, and what {dataset}` means there.

Thanks!

@jonthegeek jonthegeek merged commit 74d52e8 into master Oct 30, 2024
@jonthegeek jonthegeek deleted the LG-update-PR-instructions branch October 30, 2024 10:45
@lgibson7
Copy link
Member Author

Thanks Jon! Yeah, the visual editor made some wonky changes. I deleted the yaml it made but missed the tags.

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.

2 participants