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

Issue #58 Make snake case name functions too #90

Merged
merged 15 commits into from
Jan 8, 2019
Merged

Issue #58 Make snake case name functions too #90

merged 15 commits into from
Jan 8, 2019

Conversation

sgetalbo
Copy link
Contributor

create_any_project aesthetic function names added to the end of the four function scripts as per Steph's first comment.

@sgetalbo sgetalbo requested a review from maelle November 22, 2018 20:24
@codecov-io
Copy link

codecov-io commented Nov 22, 2018

Codecov Report

Merging #90 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #90   +/-   ##
=======================================
  Coverage   67.33%   67.33%           
=======================================
  Files          12       12           
  Lines         548      548           
=======================================
  Hits          369      369           
  Misses        179      179
Impacted Files Coverage Δ
R/createTrainingProject.R 97.56% <ø> (ø) ⬆️
R/createBasicProject.R 100% <ø> (ø) ⬆️
R/createAnalysisProject.R 100% <ø> (ø) ⬆️
R/createPackageProject.R 86.2% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0fd68d...e9d83c5. Read the comment docs.

Copy link
Contributor

@maelle maelle left a comment

Choose a reason for hiding this comment

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

Thank you!

  1. Can you run devtools::document()?
  2. Can you confirm that if you install the package locally, these new functions are available?

@maelle
Copy link
Contributor

maelle commented Dec 4, 2018

  • Can you please remove the DS_store files?

  • I don't see the Rd and NAMESPACE updates actually, did you forget to commit them?

@sgetalbo
Copy link
Contributor Author

Hi @maelle - me and Steph fixed my git woes and I have removed the DS_store file and updated the master gitignore to include that so it won't crop up again. Also - .Rd files generated fine when I re-ran the devtools::document() command after I fixed my warnings, but even though I get a 'writing NAMESPACE' alert in my console, no new namespace file appears for me to push... any ideas?

@maelle
Copy link
Contributor

maelle commented Dec 21, 2018

👋 here!

  • it seems there are a few conflicts. we can resolve the one for tests/README.md at the very end but for the other ones please try to solve them (maybe by merging master into your branch).

  • NAMESPACE was not updated. I think you need a @export roxygen2 tag before each new function, and then run document() again.

  • There are two .DS_store files to be deleted.

Thank you!

@maelle
Copy link
Contributor

maelle commented Dec 22, 2018

Thanks! Now the only issue is that there are two .DS_store files to be deleted.

@sgetalbo sgetalbo changed the title issue 58 : aesthetic function names added to end of each script Issue #58 Make snake case name functions too Jan 8, 2019
@maelle
Copy link
Contributor

maelle commented Jan 8, 2019

@sgetalbo thanks! can you resolve the conflicts? after that the PR should be ready to be merged.

@maelle maelle merged commit 9571364 into master Jan 8, 2019
@maelle maelle deleted the Issue58_ET branch January 8, 2019 15:00
maelle pushed a commit that referenced this pull request Jan 16, 2019
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

4 participants