Skip to content

deps: upgrade mocha - #6121

Closed
agungjati wants to merge 1 commit into
expressjs:masterfrom
agungjati:master-fix/upgrade-mocha
Closed

deps: upgrade mocha#6121
agungjati wants to merge 1 commit into
expressjs:masterfrom
agungjati:master-fix/upgrade-mocha

Conversation

@agungjati

@agungjatiagungjati commented Oct 31, 2024•
edited by UlisesGascon
Loading

Copy link
Copy Markdown
Contributor

@agungjatiagungjati mentioned this pull request Oct 31, 2024
Closed
@UlisesGasconUlisesGascon self-assigned this Nov 1, 2024
@UlisesGasconUlisesGascon changed the title master-fix/upgrade-mochadeps: upgrade mochaNov 1, 2024
wesleytodd
wesleytodd requested changes Jan 8, 2025
View reviewed changes

@wesleytoddwesleytodd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we are going to update this can we move to ^ so we can stop this madness 🤣?

sheplu
sheplu requested changes Jan 9, 2025
View reviewed changes

@sheplusheplu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Agree with @wesleytodd - but we should also do a global update not "per deps"

@wesleytodd
wesleytoddforce-pushed the master-fix/upgrade-mocha branch from adb5592 to 1d5129fCompareFebruary 12, 2025 16:50
@socket-security

socket-securityBot commented Feb 12, 2025

Copy link
Copy Markdown

Updated and removed dependencies detected. Learn more about Socket for GitHub ↗︎

PackageNew capabilitiesTransitivesSizePublisher
npm/mocha@10.8.2 🔁 npm/mocha@10.2.0None+695.19 MBvoxpelli

View full report↗︎

wesleytodd
wesleytodd approved these changes Feb 12, 2025
View reviewed changes

@wesleytoddwesleytodd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I updated the pr to use ^ and add history entry. @sheplu mind taking a look and approving so we don't block?

@wesleytodd

wesleytodd commented Feb 12, 2025

Copy link
Copy Markdown
Member

but we should also do a global update not "per deps"

Ah, I re-read this and accidentally didn't address this. I am not opposed to this but also don't feel too strongly either way. Since I already merged one of these PRs I guess I should stop until we decide which way we want to do this kind of update. I will wait for your reply before merging the rest and then we can adjust and force push if necessary.

sheplu
sheplu approved these changes Feb 12, 2025
View reviewed changes

@sheplusheplu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@wesleytodd

wesleytodd commented Feb 13, 2025

Copy link
Copy Markdown
Member

@sheplu and I discussed this and I am going to group all the other PR's into a single one so we reduce the noise in the repo and the history. I will use all the same commits, just rebase then squash them so @agungjati keeps the authorship.

wesleytodd pushed a commit that referenced this pull request Feb 13, 2025
@agungjati@wesleytodd
fix (deps): update deps
af44fa3
fix(deps): mocha@^10.7.3 (closes#6121)
fix(deps): marked@^15.0.3 (closes#6120)
fix(deps): express-session@^1.18.1 (closes#6119)
fix(deps): ejs@^3.1.10 (closes#6117)
fix(deps): content-type@^1.0.5 (closes#6115)
fix(deps): connect-redis@^8.0.1 (closes#6114)
fix(deps): supertest@^7.0.0 (closes#6112)
@wesleytoddwesleytodd mentioned this pull request Feb 13, 2025
Merged
wesleytodd pushed a commit that referenced this pull request Feb 13, 2025
@agungjati@wesleytodd
fix (deps): update deps
37c4799
fix(deps): mocha@^10.7.3 (closes#6121)
fix(deps): marked@^15.0.3 (closes#6120)
fix(deps): express-session@^1.18.1 (closes#6119)
fix(deps): ejs@^3.1.10 (closes#6117)
fix(deps): content-type@^1.0.5 (closes#6115)
fix(deps): connect-redis@^8.0.1 (closes#6114)
fix(deps): supertest@^6.3.4 (closes#6112)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesleytoddwesleytoddwesleytodd approved these changes
@sheplusheplusheplu approved these changes
@UlisesGasconUlisesGasconUlisesGascon approved these changes
@bjohansebasbjohansebasbjohansebas approved these changes
+1 more reviewer
@IamLizuIamLizuIamLizu approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@UlisesGasconUlisesGascon

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@agungjati@wesleytodd@sheplu@UlisesGascon@IamLizu@bjohansebas