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

Discussion: separate set_package and set_package_path methods #140

Closed
jayqi opened this issue Nov 30, 2018 · 1 comment
Closed

Discussion: separate set_package and set_package_path methods #140

jayqi opened this issue Nov 30, 2018 · 1 comment

Comments

@jayqi
Copy link
Collaborator

jayqi commented Nov 30, 2018

While reviewing #129, @jameslamb had the idea:

ok next idea related to our ongoing discussion today re #106 ....

I think we should make reporters have a set_package and set_package_path separately. DependencyReporter, for example, shouldn't have to care about pkg_path.

Thoughts?

@jayqi jayqi added the question label Nov 30, 2018
@jayqi jayqi changed the title Discussion: separate set_package and set_package_path methods Discussion: separate set_package and set_package_path methods Nov 30, 2018
@jameslamb jameslamb added the ux label Feb 10, 2019
@jayqi
Copy link
Collaborator Author

jayqi commented Feb 11, 2019

I have two ideas for how to do this:

Idea 1 : Explicit set_package_path method

Only FunctionReporter gets this method. Other ones don't. set_package no longer takes pkg_path as an argument.

Since this breaks the identical interface, but we still want to be able to pass it through in CreatePackageReport, we can do something like:

if (`set_package_path` %in% names(reporter)) {
    reporter$set_package_path(pkg_path)
}

So it will only do it for reporters where it finds the method.

Idea 2: Use ... to allow extraneous inputs

set_package has the interface function(pkg_name, ...) so it can take extra arguments without complaining.

Only FunctionReporter will have the interface function(pkg_name, pkg_path, ...). The method will call the inherited super$set_package and then also do its pkg_path stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants