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

Release 15.11.0 #7243

Closed
4 tasks done
mattxwang opened this issue Oct 16, 2023 · 14 comments
Closed
4 tasks done

Release 15.11.0 #7243

mattxwang opened this issue Oct 16, 2023 · 14 comments
Labels
status: wip is being worked on by someone

Comments

@mattxwang
Copy link
Member

mattxwang commented Oct 16, 2023

I know we're still in the middle of preparing the next major version, but we've recently merged a few changes that affect main; looking at #7212,

  • the new ignoreRules option for max-nesting-depth
  • 4 (!) different fixes for declaration-block-no-redundant-longhands (at this point, my mortal enemy 😓 )

Once we merge in #7242, I think it's a good time for us to cut a release. What are our thoughts? I'm happy to manage as much of the process as possible (I'll just need someone to review my PRs in the other repos).

checklist

  • stylelint release
  • stylelint-config-recommended update/release
  • stylelint-config-standard update/release
  • stylelint.io update
  • stylelint-demo check
  • tweet
@mattxwang mattxwang added the status: needs discussion triage needs further discussion label Oct 16, 2023
@ybiquitous
Copy link
Member

Agree with the new release. 👍🏼

And we can include #7234 too.

@Mouvedia
Copy link
Member

Mouvedia commented Oct 17, 2023

What are our thoughts?

I think main should also have #7237.

@mattxwang mattxwang added status: wip is being worked on by someone and removed status: needs discussion triage needs further discussion labels Oct 17, 2023
@mattxwang
Copy link
Member Author

All the discussed PRs have been merged, so I'm starting the process now. Marking as WIP.

@mattxwang
Copy link
Member Author

@ybiquitous have the branch protections on main (or other permissions) changed since I last released? I'm unable to git push after running npm run release, with the following output:

$ git push
Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
remote: error: GH006: Protected branch update failed for refs/heads/main.
remote: error: Changes must be made through a pull request. 10 of 10 required status checks are expected.
To github.com:stylelint/stylelint.git
 ! [remote rejected]     main -> main (protected branch hook declined)
error: failed to push some refs to 'github.com:stylelint/stylelint.git'

This has happened a handful of times already when I've tried to cut releases, so not sure exactly what's going on...

@mattxwang
Copy link
Member Author

To temporarily resolve this, I toggled the "Do not allow bypassing the above settings" option in the branch protection settings (it's now back to its original setting). I've now published 8914d46 and the related tags, but I'm thinking that this is probably not a good path forward? Any thoughts?

@mattxwang
Copy link
Member Author

Sorry for a bit of spam, but want to be transparent. In terms of tasks:

  • noticed that for other PRs, releases have just merged in the stylelint.io change w/out a review - so I've done that too
  • did the demo check
  • Tweeted

I think this release is done, but I'll leave this up for a moment in case others want to comment?

@ybiquitous
Copy link
Member

@mattxwang Great. Thank you for your work! 👏🏼

I'm thinking that this is probably not a good path forward? Any thoughts?

Make sense. The current rule is too strict when releasing. And I recommend migrating to rulesets, which have several advantages than protection rules.

@ybiquitous
Copy link
Member

ybiquitous commented Oct 17, 2023

I've created a ruleset to try out (still disabled). Can you take a quick look?

@mattxwang
Copy link
Member Author

I've created a ruleset to try out (still disabled). Can you take a quick look?

Thanks! To be honest, I'm not super familiar with rulesets. I did take a look at the GitHub Docs, and in "Rule layering", it says the following:

If the same rule is defined in different ways across the aggregated rulesets, the most restrictive version of the rule applies. As well as layering with each other, rulesets also layer with protection rules targeting the same branch or tag.

My understanding is that this might interact with "Do not allow bypassing the above settings", which is the "most restrictive" version - so perhaps this ruleset doesn't fix the core problem (which is bypassing the restriction on admins from pushing to main)? However, I might be missing something (as I mentioned, I'm not familiar with rulesets at all). I'm also happy to be a guinea pig in case you want to "try" this ruleset on me for the next release?

@mattxwang
Copy link
Member Author

mattxwang commented Oct 18, 2023

(also, will close this issue now as the release is fully completed)

@ybiquitous
Copy link
Member

@mattxwang Your concern is right.

If we continue the "most restrictive" setting (that is, forbid anyone to bypass), we should make the "Bypass list" of the ruleset empty. This is the safest against any potential missed operations.

I'll dig into the rulesets more. Since the ruleset feature shows more helpful error messages than protection rules, I think it's valuable to migrate.

@ybiquitous
Copy link
Member

ybiquitous commented Oct 18, 2023

@mattxwang I've migrated copied all rules from the branch protection to the ruleset (still disabled). Our new operation with the ruleset should be as follows:

  1. npm run release -> git push should fail due to the ruleset ("Require a pull request before merging").
  2. Disable the ruleset temporarily. (Active -> Disabled on GitHub.com)
  3. Re-run git push --follow-tags.
  4. Re-activate the ruleset.

It also may be good to disable the ruleset before npm run release. Any thoughts?

@mattxwang
Copy link
Member Author

@mattxwang I've migrated copied all rules from the branch protection to the ruleset (still disabled). Our new operation with the ruleset should be as follows:

  1. npm run release -> git push should fail due to the ruleset ("Require a pull request before merging").
  2. Disable the ruleset temporarily. (Active -> Disabled on GitHub.com)
  3. Re-run git push --follow-tags.
  4. Re-activate the ruleset.

It also may be good to disable the ruleset before npm run release. Any thoughts?

This sounds good to me! I'm happy to run through the next release and not disable the ruleset, to confirm that step 1 happens (the git push should fail due to the ruleset). How does that sound?

@ybiquitous
Copy link
Member

Good. I've just enabled the ruleset. Please do that if you perform the next release. 👌🏼

Note the old protection rule is still alive. After a while, if it's OK, let's remove it.

@ybiquitous ybiquitous mentioned this issue Nov 10, 2023
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone
Development

No branches or pull requests

3 participants