Skip to content
This repository has been archived by the owner on Jan 2, 2025. It is now read-only.

Rename R function from AppendSlopeFeatures to append_slope_features #25

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

jayqi
Copy link
Contributor

@jayqi jayqi commented Aug 14, 2019

I propose we rename the R function AppendSlopeFeatures to append_slope_features to match the interface of the Python package.

Lower-snake-case is also a commonly used style in R, e.g., see Tidyverse style guide, so I think most R users may find this change to even be an improvement.

This PR makes progress towards #13

@@ -16,9 +15,9 @@
#' in degrees. If not supplied, this will be calculated
#' from consecutive observations}
#' }
#' @param hostName A string with the host running groundhog. By default, this
Copy link

@Haoen-Cui Haoen-Cui Aug 15, 2019

Choose a reason for hiding this comment

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

@jayqi: can you ignore white space changes?
Also occurred multiple times elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Haoen-Cui if you click on the Gear icon when viewing the diff, you can hide whitespace changes in the UI.

Typically, we consider not-having-trailing-whitespace to be a good best practice, and we encourage people to turn on automatic-stripping in RStudio and other text editors.

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah it's totally fine to have your editor touch these other lines. I'm cool with it.

Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

I totally support this. Thanks for contribution @jayqi !

@@ -16,9 +15,9 @@
#' in degrees. If not supplied, this will be calculated
#' from consecutive observations}
#' }
#' @param hostName A string with the host running groundhog. By default, this
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah it's totally fine to have your editor touch these other lines. I'm cool with it.

@jameslamb jameslamb merged commit 400c36f into uptake:master Aug 16, 2019
@jayqi jayqi deleted the r_function_naming branch August 17, 2019 16:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants