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

chore: add prettier as formatter #162

Merged
merged 1 commit into from
Oct 7, 2024
Merged

Conversation

marcalexiei
Copy link
Collaborator

Part of #141

  • prettier has been added as formatter and all relevant code has formatted.
  • build-and-test.yml workflow has been renamed to CI because now it also includes format check

.github/workflows/CI.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
test/rollup.config.test.ts Outdated Show resolved Hide resolved
tsconfig.spec.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Copy link
Owner

@elycruz elycruz left a comment

Choose a reason for hiding this comment

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

@marcalexiei Changes reviewed - don't worry about my comment on updating prettier config to favor single quotes - if you have the time to do soon you can, otherwise I'll merge your changes after you address my other comments and then we can update the prettier config at a later time.

src/index.ts Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@marcalexiei
Copy link
Collaborator Author

marcalexiei commented Sep 25, 2024

All review comments should have been addressed

Copy link
Owner

@elycruz elycruz left a comment

Choose a reason for hiding this comment

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

Changes are good. Merging.

@elycruz
Copy link
Owner

elycruz commented Oct 7, 2024

@marcalexiei Hey, your changes are good (thanks for them), though note: coverage went down for insertStyle function - maybe you can add coverage for it in your next PR (it appears the early return statement isn't covered (https://coveralls.io/builds/70199789)).

@marcalexiei
Copy link
Collaborator Author

though note: coverage went down for insertStyle function - maybe you can add coverage for it in your next PR (it appears the early return statement isn't covered

The PR is not merged so I assume is better that I fix the coverage issue here...
Regarding CI permission please check #164 (comment) (on the triage permission)

@elycruz elycruz merged commit 337cb2b into elycruz:main Oct 7, 2024
3 of 4 checks passed
@elycruz
Copy link
Owner

elycruz commented Oct 7, 2024

No, it's ok - You can do it in your next PR

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