Skip to content

[code-infra] Rename CI checks to be more human readable #16910

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.

Sign up for GitHub

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

Jump to bottom
Closed
JCQuintas wants to merge 10 commits into from
Closed

[code-infra] Rename CI checks to be more human readable #16910

JCQuintas wants to merge 10 commits into from

Conversation

@JCQuintas
Copy link
Member

@JCQuintas JCQuintas commented Mar 11, 2025

@JCQuintas JCQuintas added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. scope: code-infra labels Mar 11, 2025
@JCQuintas JCQuintas self-assigned this Mar 11, 2025
@mui-bot
Copy link

mui-bot commented Mar 11, 2025 •
edited
Loading

Deploy preview: https://deploy-preview-16910--material-ui-x.netlify.app/

Bundle size report

Total Size Change: 0B(0.00%) - Total Gzip Change: 0B(0.00%)
Files: 118 total (0 added, 0 removed, 0 changed)

Details of bundle changes

Generated by 🚫 dangerJS against 3923632

@JCQuintas JCQuintas requested a review from a team March 11, 2025 11:38
@JCQuintas JCQuintas marked this pull request as ready for review March 11, 2025 11:38
@JCQuintas JCQuintas changed the title [code-infra] Rename circleci checks [code-infra] Rename circleci checks to be more human readable Mar 11, 2025
@JCQuintas JCQuintas changed the title [code-infra] Rename circleci checks to be more human readable [code-infra] Rename CI checks to be more human readable Mar 11, 2025
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 11, 2025 •
edited
Loading

It seems we would need to update the required steps in GitHub.

oliviertassinari
oliviertassinari reviewed Mar 11, 2025
View reviewed changes
oliviertassinari
oliviertassinari reviewed Mar 11, 2025
View reviewed changes
oliviertassinari
oliviertassinari reviewed Mar 11, 2025
View reviewed changes
.circleci/config.yml
Comment on lines 347 to +349
- test_unit:
<<: *default-context
name: 'JSDOM tests'
Copy link
Member

@oliviertassinari oliviertassinari Mar 11, 2025

Choose a reason for hiding this comment

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

This was nice before to train us to know that pnpm test:unit runs this one.

Copy link
Member Author

@JCQuintas JCQuintas Mar 11, 2025

Choose a reason for hiding this comment

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

We would usually check the circleci log to see why it failed, and the step will tell you the exact command to run. Both our browser and jsdom tests are unit tests, so it is a bit misleading.

For context, this is based on the base-ui changes, which we agreed on code-infra to bring to other repos: mui/base-ui#1321

Copy link
Member

@Janpot Janpot Mar 12, 2025

Choose a reason for hiding this comment

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

Personally prefer the existing naming, but I value consistency more 🙂

Copy link
Member

@michaldudak michaldudak Mar 17, 2025

Choose a reason for hiding this comment

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

This was nice before to train us to know that pnpm test:unit runs this one.

In Base UI this script was also renamed to test:jsdom, so it's still consistent.

Copy link
Contributor

@romgrk romgrk Apr 9, 2025

Choose a reason for hiding this comment

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

I think we should rename those to test:unit:jsdom and test:unit:browser, to show that it's running the same thing in different environments, and change the titles accordingly.

Copy link
Contributor

@romgrk romgrk Apr 9, 2025

Choose a reason for hiding this comment

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

And I also feel that having the job names map to npm scripts like we currently do is more convenient.

JCQuintas and others added 3 commits March 11, 2025 16:38
@JCQuintas @oliviertassinari
Update .github/workflows/vale-action.yml
30474e3 
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@JCQuintas @oliviertassinari
Update .github/workflows/maintenance.yml
acfcc44 
Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@JCQuintas
Merge branch 'master' into follow-base-naming-circleci
23029d5
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 12, 2025
@github-actions
Copy link

github-actions bot commented Mar 12, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 12, 2025
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Mar 24, 2025
@github-actions
Copy link

github-actions bot commented Mar 24, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 2, 2025
@JCQuintas
Copy link
Member Author

JCQuintas commented Apr 7, 2025

@Janpot bump 😄

This can be done async as well, just change the config and this PR should be able to be merged. Everyone would need to rebase after though.

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

github-actions bot commented May 7, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@JCQuintas
Merge branch 'master' into follow-base-naming-circleci
7e00637 
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@github-actions github-actions bot added PR: out-of-date The pull request has merge conflicts and can't be merged. and removed PR: out-of-date The pull request has merge conflicts and can't be merged. labels May 7, 2025
@github-actions
Copy link

github-actions bot commented May 8, 2025

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 23, 2025
@michelengelen michelengelen added scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). and removed scope: code-infra labels Jun 3, 2025
@github-actions
Copy link

github-actions bot commented Jun 3, 2025

Please add one type label to categorize the purpose of this PR appropriately:

bug, regression, enhancement, new feature, release or dependencies

@JCQuintas JCQuintas closed this Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romgrk romgrk romgrk left review comments

@oliviertassinari oliviertassinari oliviertassinari left review comments

@michaldudak michaldudak michaldudak left review comments

@Janpot Janpot Janpot approved these changes

Assignees

@JCQuintas JCQuintas

Labels

scope: code-infra Involves the code-infra product (https://www.notion.so/mui-org/5562c14178aa42af97bc1fa5114000cd). type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@JCQuintas @mui-bot @oliviertassinari @romgrk @Janpot @michaldudak @michelengelen