Skip to content

Conversation

@magrathj
Copy link
Contributor

@magrathj magrathj commented Aug 4, 2019

Commit to address Issues: #26 - added feature to saveDataFlatfile function for #31

Adding feature to enable changing of the flat's filename. Addressing issue @31

"export with a diferent file name daattali#31"
added changes suggested by daattali in previous pull request
R/shinyform.R Outdated
),
".csv"
)
if (!is.null(storage$filename)) {
Copy link
Owner

Choose a reason for hiding this comment

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

It's more readable to start with the is.null() case rather than the !is.null(), it's just easier on the brain to do positives than negatives. And since the if statement is really just determining the base filename (not the extension), it's a bit simpler to do this:

if (is.null(...) {
  filename <- paste(...)
} else {
  filename <- storage$filename
}
filename <- paste0(filename, ".csv")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the feedback

Thanks for the feedback, makes sense.  Made changes from the review
@daattali
Copy link
Owner

daattali commented Aug 6, 2019

Please always test every line of code that gets modified. Even very small and innocent-looking changes must be tested before committing.

The variables fileName vs filename will cause this code to break

Added test file for testsavedataflatfile function to run unit tests on the function (all passed) and re-ran devtool::test() and devtool::document()
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