Skip to content

Deprecate rules #918

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
Merged
merged 2 commits into from
Oct 10, 2021
Merged

Deprecate rules #918

merged 2 commits into from
Oct 10, 2021

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Oct 3, 2021

(this should be merged via rebase, so we have separate changelog entries)

Follow up from #864

@G-Rath G-Rath requested a review from SimenB October 3, 2021 01:41
@G-Rath G-Rath force-pushed the deprecate-rules branch 3 times, most recently from ac054b4 to 725c832 Compare October 3, 2021 02:07
SimenB
SimenB requested changes Oct 4, 2021
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

I think we should just delete the old ones rather than keeping them around.

If we want to be nice to people migrating, we can add some sort of throw 'renamed to foobar' in the old rules - I don't think we should make them actually work

@G-Rath G-Rath mentioned this pull request Oct 4, 2021
Merged
@G-Rath
Copy link
Collaborator Author

G-Rath commented Oct 4, 2021

Yeah I was going to delete them in next - this way folks could update their configs (and shared config packages) to use the new names, so that it's less breaking.

But I'm not fussed - happy to just mark them as deprecated in this PR or do nothing with them 🤷

@SimenB
Copy link
Member

SimenB commented Oct 4, 2021

Ah, this is targeting main - didn't notice. I think we can just skip the extra step of deprecating in the current release - hopefully people look at changelogs when bumping majors, so they'll see the rename

@SimenB
Copy link
Member

SimenB commented Oct 9, 2021

I wanna get next major out now. Can you change this PR to just remove/rename? As long as the changelog entry is good I don't think juggling a migration is worth it. It's a simple config change for people

@G-Rath
Copy link
Collaborator Author

G-Rath commented Oct 9, 2021

@SimenB yeah I agree - sorry for the delay, got tied up with some work stuff.

I'm keen to get the new major shipped too, so will get all the ducks in a row right now.

@G-Rath G-Rath requested a review from SimenB October 9, 2021 19:23
SimenB
SimenB approved these changes Oct 10, 2021
View reviewed changes
Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

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

woo!

@SimenB SimenB merged commit fa08f09 into main Oct 10, 2021
@SimenB SimenB deleted the deprecate-rules branch October 10, 2021 09:09
@github-actions
Copy link

🎉 This PR is included in version 24.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 25.0.0-next.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimenB SimenB SimenB approved these changes

Assignees
No one assigned
Labels
released on @next released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@G-Rath @SimenB