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

set_makevars(): Truly unique variable names and acknowledge comments #23

Merged
merged 3 commits into from
Jan 11, 2015
Merged

set_makevars(): Truly unique variable names and acknowledge comments #23

merged 3 commits into from
Jan 11, 2015

Conversation

HenrikBengtsson
Copy link
Contributor

A ~/.R/Makevars file

CFLAGS=-g -O -mtune=native -Wall -Wsign-compare -Wconversion -Wno-sign-compare
FCFLAGS=-g -O -mtune=native

would give:

Error in set_makevars(c(CFLAGS = "-g -O0 -fprofile-arcs -ftest-coverage",  :
  Multiple results for CFLAGS found, something is wrong.FALSE

because string sequence CFLAGS is part of FCFLAGS. Moreover, comments were not acknowledged, e.g.

CFLAGS=-g -O -mtune=native -Wall -Wsign-compare -Wconversion -Wno-sign-compare
## CFLAGS=-g -O -mtune=native

would give the same error message.

This pull request addresses both issues.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f7b1680 on HenrikBengtsson:master into a799fb1 on jimhester:master.

jimhester added a commit that referenced this pull request Jan 11, 2015
set_makevars(): Truly unique variable names and acknowledge comments
@jimhester jimhester merged commit d342248 into r-lib:master Jan 11, 2015
@jimhester
Copy link
Member

Thanks! I was aware when I wrote it that I did not protect against matching substrings but I should have been more defensive. I was not aware that FCFLAGS was a common environment variable.

jimhester added a commit that referenced this pull request Feb 8, 2024
* increment version

* an app with box modules example

* boilerplate for box coverage test

* ready to write box support

* now works with basic box modules

* Forgot an s

Co-authored-by: Jakub Nowicki <[email protected]>

* Smaller version increment as suggested

Co-authored-by: Jakub Nowicki <[email protected]>

* got it to work!

* added support for R6 classes attached as box modules

* increment dev version

* forgot to update the unit test with R6 modules attached

* some refactoring to clean up code

* tests for klmr/box attached modules, functions, three dots, and aliases

* a little bit of cleanup

* code cleanup. finally broke the unlist puzzle

* clean up nested ((

* test: Fix box cache cleaning in tests.

* chore: Add box to suggested dependencies.

* chore: Update package version.

* added entry in NEWS.md

* Trigger CICD

* Another CI test trigger pr (#21)

* Added `replacements_box()` for `klmr/box` support.
* Extracted `traverse_R6()` from `replacements_R6()` to reuse code in `replacements_box()`.
* R6 class box modules test cases separated to handle a known R6 issues with `r-devel`. `skip_if(is_r_devel())` is used in the R6 test cases.

* Update to covr 3.6.3 (#23)

* Adjusted DESCRIPTION and NEWS.md
* Different condition to skip tests
* Bump version for release

---------

Co-authored-by: Jim Hester <[email protected]>

* box 1.2.0 released with backports compatibility fixes

---------

Co-authored-by: Jakub Nowicki <[email protected]>
Co-authored-by: Jakub Nowicki <[email protected]>
Co-authored-by: Jim Hester <[email protected]>
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.

3 participants