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

define safe variant of short_file_name #1067

Merged
merged 1 commit into from Apr 10, 2020
Merged

Conversation

@kevinushey
Copy link
Contributor

@kevinushey kevinushey commented Apr 10, 2020

Pull Request Template for Rcpp

Closes #1066. This PR comes with two pieces:

  1. A "safe" definition of short_file_name, defined and used explicitly in the Rcpp::debug namespace;

  2. A re-worked version of the "old" (+ registered) short_file_name, which now uses a static buffer. I believe the method should be effectively un-used but I figured since it's a registered routine it's best to keep the API the same and just silence the compiler warning.

Checklist

  • Code compiles correctly
  • R CMD check still passes all tests
  • Prefereably, new tests were added which fail without the change
  • Document the changes by file in ChangeLog
@eddelbuettel
Copy link
Member

@eddelbuettel eddelbuettel commented Apr 10, 2020

I think we once use proper deprecation.

We could "tag" the older unsafer variant and say it'll go in, say, 12 month. Thoughts? (That is somewhat orthogonal to the PR though of course not fully...)

@kevinushey
Copy link
Contributor Author

@kevinushey kevinushey commented Apr 10, 2020

I don't have any strong feelings re: deprecation -- I'll let you make the call. I expect removing it would go smoothly, but experience has taught me to always expect surprises.

@codecov-io
Copy link

@codecov-io codecov-io commented Apr 10, 2020

Codecov Report

Merging #1067 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1067   +/-   ##
=======================================
  Coverage   95.61%   95.61%           
=======================================
  Files          64       64           
  Lines        2782     2782           
=======================================
  Hits         2660     2660           
  Misses        122      122           
Impacted Files Coverage Δ
src/api.cpp 100.00% <ø> (ø)
@eddelbuettel eddelbuettel merged commit c655b6e into master Apr 10, 2020
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@eddelbuettel eddelbuettel deleted the bugfix/short-file-name branch Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.