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

Remove Prettier #155

Merged
merged 1 commit into from
Aug 21, 2023
Merged

Remove Prettier #155

merged 1 commit into from
Aug 21, 2023

Conversation

juntao
Copy link
Member

@juntao juntao commented Aug 20, 2023

Explanation

The prettier command in CI gives extremely unhelpfully error messages. It simply asks me to run prettier without telling me what the actual "style issue" is.

I would NOT permit prettier to make changes to my writing in any way without first knowing what it thinks the problem is.

Plus, we already have lint. I think we should remove prettier.

Copy link
Collaborator

alabulei1 commented Aug 20, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit efad771f35ba2a1868e1ab132b18fab92e028b26

Key changes:

  • The "Prettier Code" step in the CI workflow has been commented out.

Potential problems:

  • The reviewer may want to discuss with the developer the specific error messages they are seeing from Prettier and if there are any potential solutions or configurations that can help improve the error messages.
  • It is important to ensure that the existing linting setup is comprehensive and covers all necessary checks that were previously handled by Prettier. This may require a review of the linting configuration and rules.
  • The reviewer should also verify if there are any other dependencies or scripts that rely on the "format:diff" command, as removing Prettier may impact these dependencies.

Overall, it seems the developer has a valid concern regarding the error messages from Prettier, but it is necessary to ensure that the removal of Prettier does not introduce any code style or formatting issues. A discussion with the developer is recommended to clarify the reasons behind the decision and address any potential problems arising from it.

The `prettier` command in CI gives extremely unhelpfully error messages. It simply asks me to run `prettier` without telling me what the actual "style issue" is.

*I would NOT permit `prettier` to make changes to my writing in any way without first knowing what it thinks the problem is.*

Plus, we already have lint. I think we should remove prettier.

Signed-off-by: Michael Yuan <michael@secondstate.io>
@alabulei1
Copy link
Collaborator

@hydai Do you have any feedback?

Copy link
Member

@hydai hydai left a comment

Choose a reason for hiding this comment

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

I totally agree with this. We should never rely on the prettier.

@alabulei1 alabulei1 merged commit 6f784ab into main Aug 21, 2023
6 checks passed
@alabulei1 alabulei1 deleted the juntao-remove-prettier branch August 21, 2023 03:55
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