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 #20

Merged
merged 2 commits into from
Nov 3, 2023
Merged

Update readme #20

merged 2 commits into from
Nov 3, 2023

Conversation

amanda-hi
Copy link
Contributor

Overview of Pull Request

General description of the purpose for this pull request ...

Fixes #19

Main changes

  • Clean up wording in README and add details where appropriate
  • Replace use of patchwork package w/ gridExtra

Change type

Please check the relevant box(es):

Choose reviewer(s)

Reviewer by Department

Department Reviewer Change Type
Bioinformatics @amanda-hi Code, bugs, features
@stufield bugs, features
Legal @SLbmangum LICENSE
Product @kmurugesan14 Documentation
Regulatory @nmcnabbSL Documentation

@amanda-hi amanda-hi requested a review from stufield November 1, 2023 19:28
Copy link
Contributor

@stufield stufield left a comment

Choose a reason for hiding this comment

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

Minor comments. Looks good! Still unsure what to do with patchwork. It seems to get the margins a little better than grid.arrange().

Another option ... since this is just the README, is to simply hide ALL the code and use patchwork under the hood invisibly. The README is supposed to only highlight the functionality, and how to install, etc. Not necessarily go into coding detail. So perhaps consider using more description in the text, and revealing only the code you want to use to illustrate your point. The vignettes could be used for more detail about code, and we could link to the vignettes in the README. Just a thought 🤔

@amanda-hi amanda-hi force-pushed the update-readme branch 4 times, most recently from ae9f074 to c7cb065 Compare November 3, 2023 21:33
- replaced all instances of patchwork (in vignettes) with gridExtra and added gridExtra
  to 'Suggests' in the DESCRIPTION file
- gridExtra has less dependencies and is familiar, stable, and commonly
  used in R plotting packages
- made minor verbiage modifications to README
- added details to plot table in README
- updated WORDLIST to accomodate terms used in new README plot table
- fixes #19
@amanda-hi amanda-hi merged commit f090096 into main Nov 3, 2023
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.

Review internal code chunks in README
2 participants