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

remove get_counts #72

Merged
merged 1 commit into from
Jun 19, 2018
Merged

remove get_counts #72

merged 1 commit into from
Jun 19, 2018

Conversation

jameslamb
Copy link
Collaborator

DON'T MERGE YET.

This change will go out after we merge #71 and release v0.3.0 to CRAN. #69 added a deprecation warning on the get_counts function that will ship to CRAN with the next release. This PR, which will be the first on the way to 0.4.0 (or maybe 1.0.0???), removes get_counts from the package. Unfortunately this didn't allow us to clean up the dependencies at all but it will simplify the package and make the philosophy of uptasticsearch clearer.

There are no tests in the diff because we never had any, which is fun.

@codecov-io
Copy link

codecov-io commented Jun 11, 2018

Codecov Report

Merging #72 into master will increase coverage by 9.11%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   86.93%   96.04%   +9.11%     
==========================================
  Files           9        8       -1     
  Lines         643      582      -61     
==========================================
  Hits          559      559              
+ Misses         84       23      -61

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 a3f1593...bbd56f2. Read the comment docs.

Copy link
Collaborator

@austin3dickey austin3dickey left a comment

Choose a reason for hiding this comment

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

LGTM but blocking the merge for now

@jameslamb jameslamb force-pushed the no_get_counts branch 2 times, most recently from 120081e to bbd56f2 Compare June 19, 2018 06:47
@jameslamb
Copy link
Collaborator Author

#71 is merged and 0.3.0 is (out on CRAN!)[https://cran.r-project.org/web/packages/uptasticsearch/index.html]

gonna merge this up as soon as travis builds

@jameslamb
Copy link
Collaborator Author

very very excited to merge this one and simplify the package. This is the first step on our path from 0.3.0 to 1.0.0!

@jameslamb jameslamb merged commit 4a276dc into uptake:master Jun 19, 2018
@jameslamb jameslamb deleted the no_get_counts branch June 23, 2018 18:16
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

3 participants