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

Wizard updates #224

Merged
merged 15 commits into from Feb 11, 2019
Merged

Wizard updates #224

merged 15 commits into from Feb 11, 2019

Conversation

clarabicalho
Copy link
Contributor

Updates needed for DDWizard deployment. Mainly:

  • Adding 'definitions' attribute to designers (these specify each formals' name, class, min, max, etc)
  • Including 'tip' attribute to all formals
  • Includes/updates tests for all of the above

Copy link
Contributor

@nfultz nfultz left a comment

Choose a reason for hiding this comment

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

LGTM

R/binary_iv_designer.R Show resolved Hide resolved
@@ -9,6 +9,7 @@ for(designer in designers){

the_designer <- get(x = designer)
has_shiny <- !is.null(attributes(the_designer)$shiny_arguments)
has_definition <- !is.null(attributes(the_designer)$definitions)
Copy link
Contributor

Choose a reason for hiding this comment

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

you might want to write helper methods for getting / setting definitions on objects.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could we add those methods instead on the DDWizard side? I don't see users needing to access these definitions in DesignLibrary, oder? @jaspercooper

Copy link
Contributor

Choose a reason for hiding this comment

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

that makes sense to me

@jaspercooper
Copy link
Contributor

happy for @nfultz to merge this once you and @clabima have resolved the above. Thanks both!

@coveralls
Copy link

coveralls commented Feb 11, 2019

Pull Request Test Coverage Report for Build 1275

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 1269: 0.0%
Covered Lines: 862
Relevant Lines: 862

💛 - Coveralls

@jaspercooper
Copy link
Contributor

@clabima let me know when I can update from master and merge this

@clarabicalho
Copy link
Contributor Author

@jaspercooper all clear on my side unless @nfultz objects. thanks!

@jaspercooper jaspercooper merged commit 2368e3b into master Feb 11, 2019
@jaspercooper jaspercooper deleted the wizard_updates branch February 11, 2019 15:17
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.

None yet

5 participants