Skip to content

feat: Reinstate lint infrastructure as GHA#396

Merged
krlmlr merged 20 commits intor-dbi:mainfrom
MichaelChirico:lint
May 2, 2025
Merged

feat: Reinstate lint infrastructure as GHA#396
krlmlr merged 20 commits intor-dbi:mainfrom
MichaelChirico:lint

Conversation

@MichaelChirico
Copy link
Copy Markdown
Contributor

This started from noticing test-lint.R is not really doing anything, and that {lintr} should probably be removed as a package dependency.

All changes to package code here are suggestions, I'm happy to revert anything.

Most of the changes are from the expect* series of linters that recommend "best practices" for {testthat} usage.

@krlmlr
Copy link
Copy Markdown
Member

krlmlr commented Mar 16, 2025

Thanks, this looks much cleaner overall.

R CMD check is unhappy: "Undefined global functions or variables: NUL"

What automation (if any) do you use to remove lints everywhere?

It will be easiest for me to merge a slew of the changes up front, this might take some time.

Comment thread R/spec-meta-bind-.R Outdated
@MichaelChirico
Copy link
Copy Markdown
Contributor Author

What automation (if any) do you use to remove lints everywhere?

Just regex replacement and multicursor mode

It will be easiest for me to merge a slew of the changes up front, this might take some time.

OK, take your time. Also happy to split it up if that makes more sense.

is_null_check <- ctx$tweaks$is_null_check
for (placeholder_fun in placeholder_funs) {
bind_values <- structure(data.frame(1L, check.names = FALSE), names = "")
bind_values <- structure(data.frame(1L), names = "")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@krlmlr is {constructive} generating these? maybe I need a devel version?

krlmlr and others added 7 commits May 1, 2025 18:00
This reverts commit 50017890d4720a3ab0c1a6cb09b64b9b3c7fd70c.
This reverts commit 1cba9e2aae4297612210e98d5f279ccddd6d3b9e.
This reverts commit 0f3c1573272974c8c2d9696830b94fafeb2a7d1b.
This reverts commit 57ad656facafb3f6afdf56039b2ac0940a9d692b.
@krlmlr
Copy link
Copy Markdown
Member

krlmlr commented May 1, 2025

Thanks. I've tweaked. Not a fan of nzchar(), and one of the toString() refactorings looked strange to me. Can you please make lintr happy? Other than that, ready to merge.

@krlmlr krlmlr changed the title Reinstate lint infrastructure as GHA feat: Reinstate lint infrastructure as GHA May 1, 2025
Comment thread .lintr.R
@krlmlr krlmlr merged commit 5aab034 into r-dbi:main May 2, 2025
@krlmlr
Copy link
Copy Markdown
Member

krlmlr commented May 2, 2025

Thanks! Need to proceed, will clean up remaining lints as needed.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants