Skip to content

Conversation

scagood
Copy link
Contributor

@scagood scagood commented Jul 17, 2024

What changes did you make?

This simply updates the CI and package.json to run the tests against the node versions that ESLint v9 supports

BREAKING CHANGE: Requires Node.js: ^18.18.0 || ^20.9.0 || >=21.1.0

Copy link

codecov bot commented Jul 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (bbcfcbf) to head (176e7cb).
Report is 25 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #484   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        24    -1     
  Lines          649       661   +12     
  Branches       250       247    -3     
=========================================
+ Hits           649       661   +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@brettz9
Copy link
Member

brettz9 commented Jul 17, 2024

LGTM, but maybe someone on the core team should review this kind of breaking change.

@brettz9
Copy link
Member

brettz9 commented Jul 17, 2024

@aladdin-add , @MichaelDeBoey , @voxpelli : Hi there... not sure who to ping, but I figured someone on the core team should be reviewing breaking changes?

Copy link
Member

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

I’m 👍 on aligning engine range with ESLint 9

What have the other plugins in this org done? Such as plugin-n?

I have been meaning to propose a change like this for eslint-utils as well as that’s the main thing holding my PR there back eslint-community/eslint-utils#204

"typescript": "^4.9.3"
},
"peerDependencies": {
"eslint": "^7.0.0 || ^8.0.0 || ^9.0.0"
Copy link
Member

Choose a reason for hiding this comment

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

Should we align here as well? Supporting all the way back to ESLint 7 but only Node 18 and newer feels a bit odd

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this is what I did in the original PR (#471)

4db7f5d

But I think that making that a separate PR for the change log was a good idea 😀

@voxpelli
Copy link
Member

Hi there... not sure who to ping, but I figured someone on the core team should be reviewing breaking changes?

Pinging me is very okay at least :)

The governance docs got stuck a bit with people having different views and as far as I remember no counter proposals being made, so we do not have a structured process for this yet.

Feel very free to give feedback on eslint-community/governance#1, either directly there or to me, would love to have a clear process for these kinds of things

@aladdin-add
Copy link
Contributor

leaving it open to let @xjamundx merge breaking changes, when he thinks it's a good time to release a major. :)

@brettz9 brettz9 added the chore label Jul 19, 2024
@voxpelli
Copy link
Member

leaving it open to let @xjamundx merge breaking changes, when he thinks it's a good time to release a major. :)

@aladdin-add Are you sure @xjamundx is still the one leading this module? I thought he handed it over to the ESLint Community because he wasn't interested in maintaining it anymore?

@aladdin-add
Copy link
Contributor

ah, I didn't know that! Do you think we can release a major version now? Are there any other breaking changes that should be included?

@aladdin-add aladdin-add requested a review from a team July 22, 2024 01:57
@scagood scagood force-pushed the node-versions-eslint-v9 branch from 3750db4 to 176e7cb Compare July 22, 2024 09:05
@scagood
Copy link
Contributor Author

scagood commented Jul 22, 2024

Node 22.5.0 has struck again! I guess I can only wait for the action to work out that 22.5.1 has been released 🤔
(nodejs/node#53902)

@voxpelli
Copy link
Member

Node 22.5.0 has struck again! I guess I can only wait for the action to work out that 22.5.1 has been released 🤔 (nodejs/node#53902)

If you tell it to use 22.5.1 specifically, then it will pass, but will have to be reverted before merging

@aladdin-add
Copy link
Contributor

It's all green now! ✅

@aladdin-add aladdin-add merged commit 8a981d2 into eslint-community:main Jul 24, 2024
@ota-meshi
Copy link
Member

It appears that the merge of this PR did not release successfully.
To release a major version, BREAKING CHANGE: must be included in the message footer, but this repository appears to have been configured to include only the PR title in the commit message, so BREAKING CHANGE: was not included.
I will revert this PR twice and try to release it.

https://github.com/semantic-release/semantic-release#commit-message-format

ota-meshi added a commit that referenced this pull request Jul 24, 2024
ota-meshi added a commit that referenced this pull request Jul 24, 2024
see
#484 (comment)

Reverts #504


BREAKING CHANGE: Requires Node.js: ^18.18.0 || ^20.9.0 || >=21.1.0
Copy link
Contributor

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@aladdin-add
Copy link
Contributor

it's already configured, not sure why not working as expected🤔:

image

@ota-meshi
Copy link
Member

ota-meshi commented Jul 24, 2024

I just changed that settings to "Pull request title and description". 😅
Before that it was set to "Pull request title".

renovate bot added a commit to tomacheese/cmcutter that referenced this pull request Jul 24, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[eslint-plugin-promise](https://togithub.com/eslint-community/eslint-plugin-promise)
| [`6.6.0` ->
`7.0.0`](https://renovatebot.com/diffs/npm/eslint-plugin-promise/6.6.0/7.0.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/eslint-plugin-promise/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/eslint-plugin-promise/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/eslint-plugin-promise/6.6.0/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/eslint-plugin-promise/6.6.0/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>eslint-community/eslint-plugin-promise
(eslint-plugin-promise)</summary>

###
[`v7.0.0`](https://togithub.com/eslint-community/eslint-plugin-promise/releases/tag/v7.0.0)

[Compare
Source](https://togithub.com/eslint-community/eslint-plugin-promise/compare/v6.6.0...v7.0.0)

- feat!: Update node versions to align with eslint v9
([#&#8203;505](https://togithub.com/eslint-community/eslint-plugin-promise/issues/505))
([09d0650](https://togithub.com/eslint-community/eslint-plugin-promise/commit/09d0650846806df7fc4ce26156865cf57e27fba6)),
closes
[#&#8203;505](https://togithub.com/eslint-community/eslint-plugin-promise/issues/505)
[/github.com/eslint-community/eslint-plugin-promise/pull/484#issuecomment-2246887433](https://togithub.com//github.com/eslint-community/eslint-plugin-promise/pull/484/issues/issuecomment-2246887433)
[eslint-community/eslint-plugin-promise#504](https://togithub.com/eslint-community/eslint-plugin-promise/issues/504)

##### BREAKING CHANGES

-   Requires Node.js: ^18.18.0 || ^20.9.0 || >=21.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/tomacheese/cmcutter).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
renovate bot added a commit to tomacheese/telcheck that referenced this pull request Jul 24, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[eslint-plugin-promise](https://togithub.com/eslint-community/eslint-plugin-promise)
| [`6.6.0` ->
`7.0.0`](https://renovatebot.com/diffs/npm/eslint-plugin-promise/6.6.0/7.0.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/eslint-plugin-promise/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/eslint-plugin-promise/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/eslint-plugin-promise/6.6.0/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/eslint-plugin-promise/6.6.0/7.0.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>eslint-community/eslint-plugin-promise
(eslint-plugin-promise)</summary>

###
[`v7.0.0`](https://togithub.com/eslint-community/eslint-plugin-promise/releases/tag/v7.0.0)

[Compare
Source](https://togithub.com/eslint-community/eslint-plugin-promise/compare/v6.6.0...v7.0.0)

- feat!: Update node versions to align with eslint v9
([#&#8203;505](https://togithub.com/eslint-community/eslint-plugin-promise/issues/505))
([09d0650](https://togithub.com/eslint-community/eslint-plugin-promise/commit/09d0650846806df7fc4ce26156865cf57e27fba6)),
closes
[#&#8203;505](https://togithub.com/eslint-community/eslint-plugin-promise/issues/505)
[/github.com/eslint-community/eslint-plugin-promise/pull/484#issuecomment-2246887433](https://togithub.com//github.com/eslint-community/eslint-plugin-promise/pull/484/issues/issuecomment-2246887433)
[eslint-community/eslint-plugin-promise#504](https://togithub.com/eslint-community/eslint-plugin-promise/issues/504)

##### BREAKING CHANGES

-   Requires Node.js: ^18.18.0 || ^20.9.0 || >=21.1.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View the
[repository job
log](https://developer.mend.io/github/tomacheese/telcheck).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzguMCIsInVwZGF0ZWRJblZlciI6IjM3LjQzOC4wIiwidGFyZ2V0QnJhbmNoIjoibWFzdGVyIiwibGFiZWxzIjpbXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@scagood scagood deleted the node-versions-eslint-v9 branch July 24, 2024 08:10
@voxpelli
Copy link
Member

It appears that the merge of this PR did not release successfully. To release a major version, BREAKING CHANGE: must be included in the message footer

Doing feat!: should be enough? The conventional commit convention says that the ! indicates a breaking change? Does the semver release flow not support that?

(I would personally be in favor of switching to a release please flow, like in eslint-community/eslint-plugin-n#305, as that catches errors like this before release)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants