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

[core] Use pretty-quick instead of custom script #34062

Merged
merged 9 commits into from Nov 14, 2022
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Aug 25, 2022

@Janpot Janpot added the core Infrastructure work going on behind the scenes label Aug 25, 2022
@mui-bot
Copy link

mui-bot commented Aug 25, 2022

Messages
📖 Netlify deploy preview: https://deploy-preview-34062--material-ui.netlify.app/

No bundle size changes

Generated by 🚫 dangerJS against 144bacf

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 7, 2022
@michaldudak
Copy link
Member

michaldudak commented Nov 10, 2022

It seems to be working well, I think we can go ahead with this one.
Since you removed the prettier.js script, you can also remove the listChangedFiles.js (unless it's used in other repositories, cc @mui/x).

@michaldudak michaldudak changed the title use pretty-quick instead of custom script [core] Use pretty-quick instead of custom script Nov 10, 2022
@cherniavskii
Copy link
Member

Looks good to me!
Although I don't really use prettier scripts - I have Prettier extension for VSCode installed and it runs prettier on save automatically 👍

@michaldudak
Copy link
Member

@cherniavskii Can you verify if you use listChangedFiles.js from the monorepo somewhere in the X codebase?

@Janpot
Copy link
Member Author

Janpot commented Nov 10, 2022

Yep, also using the vscode extension here. The main benefit of removing scripts/prettier.js and relying directly on the prettier CLI is that we would automatically adopt (in CI) any new file format prettier starts supporting as it updates. Or when dependent projects add prettier plugins (we added one for prisma files in Toolpad and had to update file extensions in the prettier script in core for it to work).

@cherniavskii
Copy link
Member

@cherniavskii Can you verify if you use listChangedFiles.js from the monorepo somewhere in the X codebase?

We don't use it

package.json Outdated Show resolved Hide resolved
scripts/jsonlint.js Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 10, 2022
@Janpot Janpot marked this pull request as ready for review November 10, 2022 16:40
@Janpot
Copy link
Member Author

Janpot commented Nov 10, 2022

Let me try this out on MUI X before we merge this

.eslintignore Outdated Show resolved Hide resolved
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2022
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 14, 2022
@michaldudak
Copy link
Member

Let me try this out on MUI X before we merge this

@Janpot let me know when it's ready to be merged.

@Janpot
Copy link
Member Author

Janpot commented Nov 14, 2022

It can be merged if mui/mui-x#6815 can be merged

@Janpot Janpot merged commit 6c0dd98 into mui:master Nov 14, 2022
@Janpot Janpot deleted the prettier branch November 14, 2022 16:01
the-mgi pushed a commit to the-mgi/material-ui that referenced this pull request Nov 17, 2022
daniel-rabe pushed a commit to daniel-rabe/material-ui that referenced this pull request Nov 29, 2022
feliperli pushed a commit to jesrodri/material-ui that referenced this pull request Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants