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.

Sign up for GitHub

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

Jump to bottom

Add usage vignette #18

Merged
merged 6 commits into from
Oct 11, 2024
Merged

Add usage vignette #18

merged 6 commits into from
Oct 11, 2024

Conversation

rcannood
Copy link
Collaborator

@rcannood rcannood commented Oct 9, 2024 •
edited
Loading

Minor changes

  • Add vignette to showcase laminr usage.

@rcannood rcannood requested a review from lazappi October 10, 2024 12:23
@rcannood rcannood changed the title add usage vignette Add usage vignette Oct 10, 2024
lazappi
lazappi approved these changes Oct 10, 2024
View reviewed changes
Copy link
Collaborator

@lazappi lazappi left a comment

Choose a reason for hiding this comment

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

I think this is fine as a demonstration but I added some suggestions for more explanatory text. Up to you whether to add that now or later (maybe open an issue if you don't?).

vignettes/usage.Rmd Outdated Show resolved Hide resolved
vignettes/usage.Rmd Show resolved Hide resolved
vignettes/usage.Rmd Outdated Show resolved Hide resolved
vignettes/usage.Rmd Outdated Show resolved Hide resolved
vignettes/usage.Rmd Show resolved Hide resolved
@rcannood
Copy link
Collaborator Author

Thanks for the valuable feedback! I updated the usage with the additional information. Could you have a look? :)

@rcannood rcannood requested a review from lazappi October 11, 2024 07:30
@rcannood rcannood merged commit a4f3c4c into main Oct 11, 2024
7 checks passed
@rcannood rcannood deleted the add-usage-vignette branch October 11, 2024 08:00
lazappi added a commit that referenced this pull request Oct 14, 2024
@lazappi
Merge remote-tracking branch 'origin/main' into use-api
6e3e7ae 
* origin/main:
  Add printing to remaining classes (#31)
  Improve error messages (#30)
  Update documentation (#29)
  Return `NULL` when a record's related field is empty (#28)
  minor fix in usage vignette (#32)
  Add usage vignette (#18)
  Add a simple unit test which queries laminlabs/lamindata (#27)
  bump action from v4.5.0 to v4 (#26)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lazappi lazappi lazappi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rcannood @lazappi