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 lifecycle.Rmd #1038

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Update lifecycle.Rmd #1038

merged 3 commits into from
Dec 14, 2023

Conversation

MikeJohnPage
Copy link
Contributor

In section 21.2 it states that the function package_version() comes from utils, but I think this is a mistake and is supposed to say that it comes from base.

I've updated the sections of relevant text to reflect this, but now I am not sure the updated footnote indicating that base should be added to imports (previously it said utils should be added) is relevant/required.

@jennybc
Copy link
Collaborator

jennybc commented Dec 14, 2023

Wow, I'm not sure how I did this! At first I thought maybe package_version() moved from utils to base?!? But of course it has not. In any case, thanks for the catch.

(Confusingly, there is utils::packageVersion() but that's not what I'm talking about in this section of the book.)

Yeah, the proposed fix isn't quite right yet, because package_version()'s presence in base, as opposed to utils, negates most of what's being said. Now that I've confirmed your hunch, you can take another crack. Or next time I come through I'll do it.

lifecycle.Rmd Outdated Show resolved Hide resolved
lifecycle.Rmd Outdated Show resolved Hide resolved
MikeJohnPage and others added 2 commits December 14, 2023 20:25
Co-authored-by: Jennifer (Jenny) Bryan <[email protected]>
Remove footnote
Copy link
Contributor Author

@MikeJohnPage MikeJohnPage left a comment

Choose a reason for hiding this comment

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

Footnote has been removed and your suggest changes committed.

@MikeJohnPage
Copy link
Contributor Author

(Confusingly, there is utils::packageVersion() but that's not what I'm talking about in this section of the book.)

Yes, this stumped me too. For a while I was convinced it was just a mistake of using snake_case instead of camelCase, but then I clicked they are different functions!

It should be okay to merge now I think. Thanks.

@MikeJohnPage
Copy link
Contributor Author

Also, this book is great, so thanks for all your hard work on it - it is appreciated 👏

@jennybc jennybc merged commit 36ec45e into hadley:main Dec 14, 2023
1 check passed
@MikeJohnPage MikeJohnPage deleted the patch-3 branch December 15, 2023 07:49
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