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

Auto-sync CI status for each package #24

Merged
merged 19 commits into from
Jul 28, 2024
Merged

Auto-sync CI status for each package #24

merged 19 commits into from
Jul 28, 2024

Conversation

romainmenke
Copy link
Member

@romainmenke romainmenke commented Jul 28, 2024

Which issue, if any, is this issue related to?

Closes #22

Is there anything in the PR that needs further explanation?

See https://github.com/romainmenke/stylelint-ecosystem-tester/tree/main?tab=readme-ov-file#packages


I worked on a separate fork so that I could more easily test everything without causing noise here.

Let me know if you want me to also setup a branch here with the changes if that makes it easier to do code review.


What I like about this approach:

  • the readme is really clear
  • the statuses are always up to date

But there are also some things I don't like.

There are ±50 workflows named latest and another ±50 named next

I don't like that the workflows UI on GitHub becomes much harder to navigate when there are so many workflows and when they all have the same name.

Screenshot 2024-07-28 at 15 59 27

This is needed to get nice labels :

Screenshot 2024-07-28 at 15 59 56

The checks do have nice names :

Screenshot 2024-07-28 at 16 01 48

Apparently GitHub sends a notification for each failing workflow.

Each time there is an issue it would ping the person who last altered the cron schedule. This is pretty random and noisy.

But maybe this is ok?

Our aim is to have this as green as possible.
Each time a package fails either we broke something in a Stylelint update, or the package author broke something. So the notification could be regarded as useful.

(I've already fixed stylelint-use-nesting)

@ybiquitous
Copy link
Member

@romainmenke Thank you for the great improvement! Overall, I love this change. 😄

I don't like that the workflows UI on GitHub becomes much harder to navigate when there are so many workflows and when they all have the same name.

I have the same opinion on this. 😅

Instead, I feel that the {package} / {stylelint_version} / test order is better. Any thoughts?

@romainmenke
Copy link
Member Author

Instead, I feel that the {package} / {stylelint_version} / test order is better.

Yes, I think that might be better :)

Screenshot 2024-07-28 at 17 38 49

What do you think of using an npm badge instead of the package name?
This would reduce the width and duplication in the table a bit.

Screenshot 2024-07-28 at 17 36 37

vs.

Screenshot 2024-07-28 at 17 37 44

@ybiquitous
Copy link
Member

Thanks. I love the npm badge idea!

package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Thanks a lot! This is a great achievement. Feel free to merge. 😄 👍🏼

@romainmenke romainmenke merged commit a39a25e into stylelint:main Jul 28, 2024
81 of 106 checks passed
@Mouvedia
Copy link
Member

Once WordPress/gutenberg#64828 is merged, we will have to change the test command for @wordpress/stylelint-config.
i.e. it's run from here

@mikeybinns
Copy link

Once WordPress/gutenberg#64828 is merged, we will have to change the test command for @wordpress/stylelint-config. i.e. it's run from here

The test:unit command would run the unit tests for all packages. To test just the stylelint package, you can do this command:

npm run test:unit -- --testPathPattern packages/stylelint-config

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.

Auto-sync CI status for each package
4 participants