Skip to content

settings not WAI in package sub-directory? #2512

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

Closed
MichaelChirico opened this issue Jan 15, 2024 · 3 comments · Fixed by #2514
Closed

settings not WAI in package sub-directory? #2512

MichaelChirico opened this issue Jan 15, 2024 · 3 comments · Fixed by #2514
Labels

Comments

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented Jan 15, 2024

In Rdatatable/data.table@5e36615, I tried to put the .lintr config into a non-standard location and point to it in CI with the env approach mentioned in ?read_settings: R_LINTR_LINTER_FILE: .ci/.lintr. Also tried with .ci/.lintr.R (the exact file, not relying on auto-guessing the R extension).

However, this failed to get picked up, as seen by the 1000s of lints picked up in the associated job: https://github.com/Rdatatable/data.table/actions/runs/7523183440/job/20476211212?pr=5908

Am I doing something wrong?

I've also tried running options(lintr.linter_file = ".ci/.lintr") (and .ci/.lintr.R) to no avail. I've also print-debugged that file.exists(".ci/.lintr.R"), so the file is not in the wrong location / omitted for being in a hidden directory.

Currently, I'm needing to run lintr:::read_settings(".ci/lintr"); lint_package(parse_settings = FALSE) to accomplish what I intended. Really not pretty.

PS it might help for verbose=TRUE to report some info in read_settings() as well, e.g. which file was chosen, maybe even which settings are read as which values.

@MichaelChirico MichaelChirico changed the title R_LINTR_LINTER_FILE not WAI? settings not WAI in package sub-directory? Jan 15, 2024
@AshesITR
Copy link
Collaborator

Any reason for using basename(...) here?

find_local_config(path, basename(linter_file))

@MichaelChirico
Copy link
Collaborator Author

I assume you're referring to:

find_local_config(path, basename(linter_file)),

IINM that entered the package long ago before the current form, then survived in #2205 without being re-examined:

1a7114b

But in short I don't know for sure what the aim is there...

@AshesITR
Copy link
Collaborator

IINM removing the call to basename() should fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants