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

Fix changelog generation #6299

Merged
merged 12 commits into from Aug 31, 2022
Merged

Fix changelog generation #6299

merged 12 commits into from Aug 31, 2022

Conversation

JounQin
Copy link
Member

@JounQin JounQin commented Aug 23, 2022

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

Related #6282

Is there anything in the PR that needs further explanation?

No, it's self-explanatory.

@JounQin JounQin added the type: infrastructure an improvement to devops label Aug 23, 2022
@JounQin JounQin requested a review from jeddy3 August 23, 2022 10:16
@JounQin JounQin self-assigned this Aug 23, 2022
@changeset-bot
Copy link

changeset-bot bot commented Aug 23, 2022

⚠️ No Changeset found

Latest commit: 7944d0d

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

package.json Outdated Show resolved Hide resolved
@JounQin
Copy link
Member Author

JounQin commented Aug 24, 2022

@ybiquitous @jeddy3 Are we good to go forward?

@jeddy3 jeddy3 removed the type: infrastructure an improvement to devops label Aug 24, 2022
@JounQin
Copy link
Member Author

JounQin commented Aug 25, 2022

friendly ping @ybiquitous @jeddy3

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.

When I run npm run version, the extra sections are not inserted. Thank you.

Instead, the - patch line is inserted. Here is an example of CHANGELOG.md diff:

@@ -1,5 +1,10 @@
 # Changelog
 
+## 14.12.0
+
+- patch
+- Added: `declaration-block-no-duplicate-properties` autofix ([#6296](https://github.com/stylelint/stylelint/pull/6296)) ([@fpetrakov](https://github.com/fpetrakov)). - minor
+
 ## 14.11.0
 
 - Added: `ignoreAfterCombinators: []` to `selector-max-universal` ([#6275](https://github.com/stylelint/stylelint/pull/6275)).

@JounQin
Copy link
Member Author

JounQin commented Aug 25, 2022

Instead, the - patch line is inserted. Here is an example of CHANGELOG.md diff:

Interesting, let me check. It seems I forget to check whether the content exists.


@ybiquitous It should be fixed.

.changeset/changelog-stylelint.js Outdated Show resolved Hide resolved
.changeset/changelog-stylelint.js Outdated Show resolved Hide resolved
.changeset/changelog-stylelint.js Outdated Show resolved Hide resolved
.changeset/changelog-stylelint.js 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. LGTM 👍🏼

I locally confirm it works:

--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,16 @@
 # Changelog
 
+## 15.0.0
+
+- Removed: `foo` rule. (BREAKING)
+- Changed: `foo` rule. (BREAKING)
+- Deprecated: `foo` rule.
+- Added: `bar` rule.
+- Added: `declaration-block-no-duplicate-properties` autofix ([#6296](https://github.com/stylelint/stylelint/pull/6296)) ([@fpetrakov](https://github.com/fpetrakov)).
+- Added: `foo` rule.
+- Fixed: `bar` rule.
+- Fixed: `foo` rule.
+
 ## 14.11.0
 
 - Added: `ignoreAfterCombinators: []` to `selector-max-universal` ([#6275](https://github.com/stylelint/stylelint/pull/6275)).

@ybiquitous ybiquitous changed the title ci: restore package.json after changeset version Fix changelog generation Aug 29, 2022
@JounQin
Copy link
Member Author

JounQin commented Aug 29, 2022

@ybiquitous Let's merge? cc @jeddy3

JounQin and others added 11 commits August 29, 2022 21:38
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
Co-authored-by: Masafumi Koba <473530+ybiquitous@users.noreply.github.com>
@ybiquitous
Copy link
Member

Let's wait for other reviews.

@JounQin
Copy link
Member Author

JounQin commented Aug 30, 2022

I think we can merge now? @ybiquitous

@ybiquitous
Copy link
Member

I'm curious about the other members' thoughts on the patch and the tricky script, so let's wait for feedback a little longer.

By the way, I notice changesets/changesets#616 was closed today. Is overrides.prettier still necessary? (ref: #6299 (comment))

@JounQin
Copy link
Member Author

JounQin commented Aug 30, 2022

I'm curious about the other members' thoughts on the patch and the tricky script, so let's wait for feedback a little longer.

It seems there are only us two invoked in this PR.

By the way, I notice changesets/changesets#616 was closed today. Is overrides.prettier still necessary? (ref: #6299 (comment))

Nice catch, I'll check ASAP. Done!

@JounQin JounQin force-pushed the ci/changesets_version branch 2 times, most recently from 3b6404f to d0a871a Compare August 30, 2022 15:26
@ybiquitous
Copy link
Member

@JounQin Thanks for resolving the Prettier problem.

I see the following diff of the package-lock.json file when running npm install locally:

--- a/package-lock.json
+++ b/package-lock.json
@@ -89,6 +89,7 @@
         "postcss-less": "^6.0.0",
         "postcss-sass": "^0.5.0",
         "postcss-scss": "^4.0.4",
+        "prettier": "2.7.1",
         "remark-cli": "^11.0.0",
         "sugarss": "^4.0.1",
         "typescript": "^4.7.4"

Could you fix the diff also, please?

@JounQin
Copy link
Member Author

JounQin commented Aug 31, 2022

Could you fix the diff also, please?

@ybiquitous Removed, although why it is added previously. 😂

@JounQin
Copy link
Member Author

JounQin commented Aug 31, 2022

I'm going to merge and wait for feedback from the other maintainers.

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