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

Revert removed changelog entry #7039

Merged
merged 1 commit into from Jul 4, 2023
Merged

Revert removed changelog entry #7039

merged 1 commit into from Jul 4, 2023

Conversation

jeddy3
Copy link
Member

@jeddy3 jeddy3 commented Jul 3, 2023

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

None.

Is there anything in the PR that needs further explanation?

We've consistently followed the conventions of:

  • Fixed & Security = Patch
  • Added & Deprecated = Minor
  • Removed & Changed = Major

This pull request removes the inconsistent changelog entry, which isn't user-facing and is "patch" for "Removed".

@changeset-bot
Copy link

changeset-bot bot commented Jul 3, 2023

⚠️ No Changeset found

Latest commit: e0811bc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@romainmenke
Copy link
Member

Removed & Changed = Major

Good to know 🙇


This was initially :

Fixed: remove `postcss-media-query-parser` dependency

#6999 (comment)

Should it be reverted to that?

@jeddy3
Copy link
Member Author

jeddy3 commented Jul 3, 2023

Should it be reverted to that?

I don't think so. It's not user-facing, and "Fixed: remove" will be inconsistent with the other entries in the changelog.

Copy link
Member

@romainmenke romainmenke left a comment

Choose a reason for hiding this comment

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

LGTM!

@ybiquitous
Copy link
Member

Um... I acknowledge the inconsistency, but will not PR #6999 affect people, especially plugin authors using postcss-media-query-parser?

I think we should inform people about the removed dependency in some way. Any thoughts?

@jeddy3
Copy link
Member Author

jeddy3 commented Jul 4, 2023

I don't think we need to as those changes were a refactor that didn't change our public API (for consumers and plugin authors). However, how about we include a second tweet as part of the release to let people know about and draw attention to the new parser? Something along the lines of:

"Under the hood, we've replaced our media query parser with @romainmenke's more modern and spec-compliant one. If you're a plugin author targeting media queries, we recommend using this parser too: https://www.npmjs.com/package/@csstools/media-query-list-parser"

That way people are informed and the changelog remains for our public API.

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.

That sounds good. 👍🏼

@jeddy3 jeddy3 merged commit 16110fd into main Jul 4, 2023
16 checks passed
@jeddy3 jeddy3 deleted the remove-changelog-entry branch July 4, 2023 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants