CSHS-CWRA / CSHShydRology

Main package
GNU Affero General Public License v3.0
35 stars 39 forks source link

Whitfield branch #7

Closed KevinShook closed 2 years ago

KevinShook commented 6 years ago

The function names in the Whitfield branch all begin with a lower-case letter, which is contradicted by the Google code standards at https://google.github.io/styleguide/Rguide.xml#identifiers:

I got the following errors when running check()

checking dependencies in R code ... WARNING
'::' or ':::' imports not declared from:
  ‘Graphics’ ‘Kendall’ ‘fields’ ‘raster’ ‘timeDate’
...
ocumented arguments not in \usage in documentation object 'HYDAT_list':
  ‘stnID’ ‘stn’

Undocumented arguments in documentation object 'doys'
  ‘Date’
Documented arguments not in \usage in documentation object 'doys':
  ‘{Date}’

Functions with \usage entries need to have the appropriate \alias
entries, and all their arguments documented.
The \usage entries must correspond to syntactically valid R code.
See chapter ‘Writing R documentation files’ in the ‘Writing R
Extensions’ manual.
boshek commented 6 years ago

From the style guide I see this:

variable.name is preferred, variableName is accepted GOOD: avg.clicks OK: avgClicks BAD: avg_Clicks

So I think Paul's naming is fine. Also I think this type of comment likely should reside directly in the pull request.

KevinShook commented 6 years ago

Hi Sam, That's for variable names. According to the style guide

function names have initial capital letters and no dots (FunctionName); constants are named like functions but with an initial k.

K

On 2018-06-13 09:36 AM, Sam Albers wrote:

From the style guide I see this:

variable.name is preferred, variableName is accepted GOOD: avg.clicks OK: avgClicks BAD: avg_Clicks

So I think Paul's naming is fine. Also I think this type of comment likely should reside directly in the pull request.

- You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/CSHS-CWRA/CSHShydRology/issues/7#issuecomment-396982778, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AKieTXw4WpkCbEjkYAhHzJjRXVhR-i-Aks5t8TGKgaJpZM4UlbMy.

-- Dr. Kevin Shook, P.Eng.

Research Scientist

Centre for Hydrology, University of Saskatchewan 101-121 Research Dr. Saskatoon, SK S7N 1K2

Phone: (306)966-5514

boshek commented 6 years ago

Right. I do that convention I think would put us out of step with most of the R world. Most functions I see these days follow the all lower case and using _ conventions.

PaulWhitfield commented 2 years ago

all these now resolved