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

Added draft lintr script #69

Merged
merged 15 commits into from
Jan 16, 2024
Merged

Added draft lintr script #69

merged 15 commits into from
Jan 16, 2024

Conversation

rmbielby
Copy link
Contributor

@rmbielby rmbielby commented Jan 12, 2024

Pull request overview

Just adding the lintr script, which performs overlapping things with tidy_code() / stylr, but some extra things too. Tried it out on the attendance dashboard and it looks like it potentially over-flags a bit, but putting it here so we can have a look through how it handles the template.

There's a handy guide on setting up lintr in an R Project at this page: https://lintr.r-lib.org/articles/lintr.html

@rmbielby rmbielby self-assigned this Jan 12, 2024
@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

# publication name (e.g. the EES publication)
ees_pub_name <- "Statistical publication"
# Update with parent publication link
ees_publication <- "https://explore-education-statistics.service.gov.uk/find-statistics/"

Check notice

Code scanning / lintr

Lines should not be more than 80 characters. This line is 89 characters. Note

Lines should not be more than 80 characters. This line is 89 characters.
@@ -104,7 +112,8 @@
choicesAreas <- dfAreas %>%
filter(geographic_level == "National") %>%
select(geographic_level, area_name = country_name) %>%
rbind(dfAreas %>% filter(geographic_level == "Regional") %>% select(geographic_level, area_name = region_name)) %>%
rbind(dfAreas %>% filter(geographic_level == "Regional") %>%

Check notice

Code scanning / lintr

%>% should always have a space before it and a new line after it, unless the full pipeline fits on one line. Note

%>% should always have a space before it and a new line after it, unless the full pipeline fits on one line.
@@ -104,7 +112,8 @@
choicesAreas <- dfAreas %>%
filter(geographic_level == "National") %>%
select(geographic_level, area_name = country_name) %>%
rbind(dfAreas %>% filter(geographic_level == "Regional") %>% select(geographic_level, area_name = region_name)) %>%
rbind(dfAreas %>% filter(geographic_level == "Regional") %>%
select(geographic_level, area_name = region_name)) %>%

Check notice

Code scanning / lintr

Indentation should be 10 spaces but is 4 spaces. Note

Indentation should be 10 spaces but is 4 spaces.
@rmbielby
Copy link
Contributor Author

I've left a few lintr warnings active to illustrate the kind of things it's picking up. Most of them are pretty quickly fixable if we do want to follow the lintr guidance as much as we possibly can. Think there's just the long lines that are effectively just down to us including a url that are the ones I wouldn't be able to fix in the code. Although I did see something about ignore rules in lintr somewhere. Might be that those can be written to ignore any line with http in it.

@rmbielby rmbielby requested a review from cjrace January 15, 2024 16:19
Copy link
Contributor

@cjrace cjrace left a comment

Choose a reason for hiding this comment

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

Happy to merge this in and see how it goes / keep it under review

@rmbielby rmbielby merged commit b25ed5d into main Jan 16, 2024
4 checks passed
@cjrace cjrace deleted the add-lintr branch August 6, 2024 16:45
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