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

A big lint #6127

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

A big lint #6127

wants to merge 28 commits into from

Conversation

teunbrand
Copy link
Collaborator

@teunbrand teunbrand commented Oct 1, 2024

This PR makes a lot of small tweaks in the code based on a select few linters from the {lintr} package.

I didn't blindly obey all the linters; in some cases, the linter has been selectively ignored.
For example I didn't reindent/rewhitespace the .element_tree construction because the whitespace facilitates vertical alignment.
In other cases, I've chosen not to include some linters, like the cyclomatic complexity one, as ggproto classes were being counted as 1 big function.

I tried to group similar changes in the same commit so it is easy to undo some changes globally.

Copy link
Member

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

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

Have you aligned this with the tidy style guide? I'm not sure lintr is fully reflective of that?

@teunbrand
Copy link
Collaborator Author

No but I think lintr and tidyverse style guide share a lot of common ground.
I'm happy to either (1) remove commits from this PR that introduce changes you think are for the worse or (2) pass code through styler

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.

2 participants