Skip to content

Closes #97 Add {whirl} to logging article #98

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

akselthomsen
Copy link

@akselthomsen akselthomsen commented Apr 24, 2025

Pull Request

I have added whirl to the comparison article of the different logging packages (currently log and logrx).

In order to not change the article too much I have mostly added whirl in the existing sections, and then I added one extra section on how to execute multiple scripts with whirl.

Note that I am executing the log generation inside the Quarto document to make sure it always reflects the latest version of whirl. I see that is not the case for logr and logrx. I will be happy to update their examples to do the same, or change mine to also use a static log.
I have tested this by building webpage locally with all the latest package versions.

I was not able to link an issue, but have created #97 for his PR.

Before you submit your pull request, take a look at the following checklist. Many thanks for your contribution!

  • Title: Place Closes #<insert_issue_number> at the beginning of your PR title. Use the Edit button in the top-right if you need to update.
  • Linked Issue: Ensure the related issue is linked in the "Development Section" on the right-hand side.
  • First Contribution: If this is your first contribution, add yourself to the DESCRIPTION file.
  • Impact on Examples: If your updates impact any examples, review locally for warnings or errors in the impacted example pages.
  • Merge Conflicts: Developers should address any merge conflicts and merge upon successful review.
  • New Packages: If new packages were used, ensure they are included in the DESCRIPTION file's Imports section.
  • Updated Examples: If you added or updated an example, ensure it runs on the latest CRAN release versions of all packages used.
  • Testing Instructions: Provide instructions on how to test the code if necessary.

@akselthomsen
Copy link
Author

@bms63 tagging for the review :)
Btw, I have also opened an issue to add whirl to the pharmaverse (pharmaverse/pharmaverse#376).

@@ -40,7 +41,8 @@ Imports:
tidyr,
xportr,
logr,
logrx
logrx,
whirl
Copy link
Collaborator

Choose a reason for hiding this comment

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

so excited to see this added!

Copy link
Collaborator

@bms63 bms63 left a comment

Choose a reason for hiding this comment

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

I am loving this read!! @rossfarrugia FYI

I need to generate the site locally to review, but my first pass gives it an A+

@bms63 bms63 requested a review from rossfarrugia April 25, 2025 01:24
@rossfarrugia rossfarrugia linked an issue Apr 28, 2025 that may be closed by this pull request
@rossfarrugia
Copy link
Contributor

thanks both - will take a look in the next week or 2 and try to coordinate this example publishing along with hopefully getting the package added to pharmaverse

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.

Add {whirl} to logging article
3 participants