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

Add image functions #15

Merged
merged 7 commits into from
Apr 28, 2023
Merged

Add image functions #15

merged 7 commits into from
Apr 28, 2023

Conversation

bcslau
Copy link
Collaborator

@bcslau bcslau commented Apr 28, 2023

Addition of image functions get_images.R and save_images.R. Images extracted from inst folder, which currently only contains 2 microsoft images.

@bcslau bcslau linked an issue Apr 28, 2023 that may be closed by this pull request
Copy link
Member

@Paulj1989 Paulj1989 left a comment

Choose a reason for hiding this comment

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

The branch changes failed the initial checks (automated by GitHub Actions) due to some minor issues:

  • dplyr and magrittr were not specified as package imports, but several dplyr functions and the magrittr pipe (%>%) are called in the get_images() function.
  • The Images column from the gt table was called as an object, which was throwing up a NOTE (low-level warning) because the checks were looking for a global variable that wasn't there.

I made the following changes to fix these issues and fixed a couple formatting inconsistencies (rather than suggesting changes and leaving this here until you return from leave):

  • Replaced %>% with the base R pipe (|>) which has the same functionality without requiring magrittr as a dependency.
  • Added dplyr to the package imports
  • Wrapped Images in single-quotes so that devtools::check() doesn't think it is a global variable.
  • Took out the @return and @examples tags from the roxygen blocks for both functions as they are currently empty (worth filling later).
  • Renamed the Brand_images folder to images and changed the image names to lowercase.

The branch changes are now passing all checks so it is ready to be merged with main.

@Paulj1989 Paulj1989 added the enhancement New feature or request label Apr 28, 2023
@Paulj1989 Paulj1989 merged commit 6985e3e into main Apr 28, 2023
@Paulj1989 Paulj1989 deleted the Add_image_functions branch April 28, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make it easier to add SCW brand images to a plot
2 participants