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

Adding consistent formatting rule for react server #222

Closed
wants to merge 4 commits into from

Conversation

massi-ang
Copy link
Contributor

Resolves partially #221

With this PR I would like to improve the maintainability of this awesome project and simplify public contributions.

This PR does not add or modify any functionality from the current code and just reformats the code.

This is a preparatory PR to enable Linting in order to have an easily reproducible change that can be verified.

The changes in this PR can be reproduced by executing:

git checkout eaaf10c
npm install
npx prettier -w .

@ianarawjo
Copy link
Owner

ianarawjo commented Feb 17, 2024

Thanks for the suggestion Massi, I've implemented this change in the branch ianarawjo:prettier. Please git rebase your Issue for eslint (#223) on top of the prettier branch. I can then merge the changes into the main branch after verifying.

The reason I needed to create my own branch for prettier and close this Issue instead of merging was twofold:

  • I cannot review all the changes in this PR manually. Even though I trust you I want to err on side of caution and reproduce prettier myself (using your configs)
  • I wanted to verify that the code ran after prettier. It turned out that I needed to cleanup some lines in backend.ts and utils.ts which were annotated with @ts-ignore, as these lines use meta-programming/access variables that may not be declared or type-safe.

(Note that it may be intractable to rebase; if so, you would have to copy over your eslint config and reproduce the changes on the new branch. Sorry about that!)

@ianarawjo ianarawjo closed this Feb 17, 2024
@massi-ang
Copy link
Contributor Author

massi-ang commented Feb 19, 2024

Fantastic. I do understand your rationale and I would have done the same. This is also why I explained how to reproduce the changes (and yes, I forgot to mention the @ts-ignore line changes 😞 )

@massi-ang massi-ang deleted the chore_formatting branch February 19, 2024 09:16
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.

None yet

2 participants