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

Introduce prettier to format JavaScript (.js) and Sass (.scss) files #594

Closed
wants to merge 3 commits into from

Conversation

TymekDev
Copy link
Contributor

@TymekDev TymekDev commented Jun 17, 2024

Description

With this PR I introduce two new functions: format_js() and format_sass(). Both of these take a fix argument that defaults to TRUE to allow just checking the style.

Important

There might be places where prettier's formatting might be incompatible with AirBnB and/or stylelint linters, but I am not aware of these.

How to Test

A New App

  1. Install Rhino from the branch
  2. Create a new app with rhino::init()
  3. Add some unformatted code in app/js/ and app/styles/ directories
  4. Run format_js() and format_sass() to see if they work

An Existing App

  1. Run renv::install("TymekDev/rhino@formatters")
  2. Remove .rhino/ directory
  3. Make sure there is unformatted code in app/js/ and app/styles/ directories
  4. Run format_js() and format_sass() to see if they work

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())

@TymekDev TymekDev requested a review from kamilzyla June 17, 2024 09:21
@TymekDev TymekDev marked this pull request as ready for review June 17, 2024 09:37
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, please address my comments and you are ready to go.

Copy link
Member

Choose a reason for hiding this comment

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

Please bum the package version.

# rhino 1.8.0
# rhino (development version)

1. Introduce `format_js()` and `format_sass()` powered by [prettier](https://prettier.io).
Copy link
Member

Choose a reason for hiding this comment

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

Please add an entry for the migration guide (remove .rhino etc.)

@kamilzyla
Copy link
Collaborator

Merged as #597.

@kamilzyla kamilzyla closed this Jul 8, 2024
@TymekDev TymekDev deleted the formatters branch July 8, 2024 11:27
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.

3 participants