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

Style box use #114

Merged
merged 22 commits into from
Jul 12, 2024
Merged

Style box use #114

merged 22 commits into from
Jul 12, 2024

Conversation

radbasa
Copy link
Collaborator

@radbasa radbasa commented Jul 4, 2024

Description

  • All packages are called under one box::use().
  • All modules are called under one box::use().
  • Package and module levels are re-formatted to multiple lines. One package per line.
  • Packages and modules are sorted alphabetically, ignoring the aliases.
  • Functions attached in a single line retain the single line format.
  • Functions attached in multiple lines retain the multiple line format.
  • Functions are sorted alphabetically, ignoring the aliases.
  • A trailing comma is added to packages, modules, and functions (optional via function argument).
  • Retains inline comments. >>> # nolint.
  • Non-box::use() commands found between box::use() calls are moved to the top of the file
  • CLI status output

Technical notes

  • treesitter was used for code parsing
  • sprintf() instead of paste() or glue::glue() for performance

Definition of Done

  • The change is thoroughly documented.
  • The CI passes (R CMD check, linter, unit tests, spelling).
  • Any generated files have been updated (e.g. .Rd files with roxygen2::roxygenise())

@radbasa radbasa marked this pull request as ready for review July 9, 2024 11:01
@radbasa radbasa requested a review from jakubnowicki July 9, 2024 11:01
@radbasa radbasa changed the base branch from dev_styling to main July 9, 2024 11:02
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 96.44444% with 8 lines in your changes missing coverage. Please review.

Project coverage is 97.99%. Comparing base (845e91e) to head (751db9f).

Files Patch % Lines
R/style_box_use.R 96.44% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
- Coverage   98.41%   97.99%   -0.43%     
==========================================
  Files          21       22       +1     
  Lines         821     1046     +225     
==========================================
+ Hits          808     1025     +217     
- Misses         13       21       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@jakubnowicki jakubnowicki left a comment

Choose a reason for hiding this comment

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

  1. We do not force the trailing comma inside [], it would be great to be able to control this here (e.g. via option). The default should be the same as in the trailing commas linter.
  2. styling functions throw a warning when there is no box::use statement (e.g. here)
Nothing to modify in `app/logic/general_utils.R`.
Warning messages:
1: In min(box_lines) : no non-missing arguments to min; returning Inf
2: In max(box_lines) : no non-missing arguments to max; returning -Inf

@radbasa
Copy link
Collaborator Author

radbasa commented Jul 10, 2024

  1. We do not force the trailing comma inside [], it would be great to be able to control this here (e.g. via option). The default should be the same as in the trailing commas linter.
  2. styling functions throw a warning when there is no box::use statement (e.g. here)
Nothing to modify in `app/logic/general_utils.R`.
Warning messages:
1: In min(box_lines) : no non-missing arguments to min; returning Inf
2: In max(box_lines) : no non-missing arguments to max; returning -Inf
  1. Added a trailing_commas_func argument to switch trailing commas inside []. Default is FALSE.
  2. Graceful handling of min() Inf. Not suppressWarnings().
  3. Added appropriate tests at the lower levels. Assumed trailing_commas_func = FALSE in higher/larger/e2e tests.

Copy link
Member

@jakubnowicki jakubnowicki left a comment

Choose a reason for hiding this comment

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

LGTM

@radbasa radbasa merged commit 1a31217 into main Jul 12, 2024
5 of 7 checks passed
@radbasa radbasa deleted the style-box-use branch July 12, 2024 04:24
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.

[Styler] Sort package imports in alphabetical order.
2 participants