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

Don't instruct usage if no changes in use_package() #1384

Merged
merged 10 commits into from
Apr 16, 2021
Merged

Don't instruct usage if no changes in use_package() #1384

merged 10 commits into from
Apr 16, 2021

Conversation

malcolmbarrett
Copy link
Collaborator

@malcolmbarrett malcolmbarrett commented Mar 5, 2021

If I do use_package("pre.existing.import", "Suggests"), use_dependency() warns me that no change will be made. However, how_to_use() doesn't know about this, so it still tells you how to use the given type.

This PR prevents use_package() and use_dev_package() from calling how_to_use() if no change has been made.

I thought to add a test for this little bug, but I don't know how to express the expectation "A warning, followed by no messages". Maybe just a snapshot test?

PS I actually don't understand the logic here, TBH. Why does usethis care if I'm shuffling dependencies around in the first place? And why does it let me move up the dependency ladder but not down?

Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

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

A snapshot test would be perfect I think. And add a news bullet?

R/package.R Outdated Show resolved Hide resolved
@malcolmbarrett
Copy link
Collaborator Author

Ok, style updated, test added, and news updated!

@jennybc
Copy link
Member

jennybc commented Apr 16, 2021

PS I actually don't understand the logic here, TBH. Why does usethis care if I'm shuffling dependencies around in the first place? And why does it let me move up the dependency ladder but not down?

Just to provide some history, use_dependency() is sort of a nightmare to work on, i.e. deserving of a comment "you think you will be able to make this much better, but that's what the last person thought" 😅

@malcolmbarrett
Copy link
Collaborator Author

"you think you will be able to make this much better, but that's what the last person thought"

🤣 ok, I'll always use ten_foot_pole() when dealing with use_dependency()

@hadley hadley merged commit 1e6b701 into r-lib:master Apr 16, 2021
@hadley
Copy link
Member

hadley commented Apr 16, 2021

Thanks!

@malcolmbarrett malcolmbarrett deleted the use_pkg_bail_out branch April 16, 2021 18:06
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.

3 participants