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

Update ini package to 1.3.8 & merge-deep to #3333

Merged
merged 1 commit into from
Jun 4, 2021

Conversation

deetergp
Copy link
Contributor

@deetergp deetergp commented Jun 3, 2021

Details

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/165493

Tests & QA

General regression testing on the desktop app.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

@deetergp deetergp self-assigned this Jun 3, 2021
@deetergp deetergp requested a review from a team as a code owner June 3, 2021 00:43
@botify
Copy link

botify commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@MelvinBot MelvinBot requested review from jasperhuangg and removed request for a team June 3, 2021 00:43
@jasperhuangg
Copy link
Contributor

jasperhuangg commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@deetergp I don't understand, how did you update the package-lock.json without updating the package.json? Or was the package.json already updated and we forgot to run npm install to update the package-lock.json previously?

@deetergp deetergp closed this Jun 3, 2021
@deetergp
Copy link
Contributor Author

deetergp commented Jun 3, 2021

Hey @jasperhuangg, this was a WIP and I didn't see that you got assigned. The reason I was updating the package-lock.json and not the package.json is because the ini package is an indirect dependency of a package we use. I had to update it because of this Snyk vulnerability issue, but then I realized it's a part of expensify-common and should be updated there, and then in every repo that uses the expensify-common library. So I am going to close this PR.

See this SO for more detail.

@deetergp deetergp deleted the scott-fixSNYK-JS-INI-1048974 branch June 3, 2021 22:55
@deetergp deetergp restored the scott-fixSNYK-JS-INI-1048974 branch June 3, 2021 23:00
@deetergp
Copy link
Contributor Author

deetergp commented Jun 3, 2021

And now that I've gone and said that, I realize that while it is in expensify-common, it is a dev dependency, so it doesn't really matter. But Expensify.cash uses it as a dependency of Electron, so I am going to re-open this PR, test it a bit further, then remove the WIP once I'm convinced it's okay.

@deetergp deetergp reopened this Jun 3, 2021
@botify
Copy link

botify commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

2 similar comments
@botify
Copy link

botify commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@botify
Copy link

botify commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@deetergp deetergp changed the title [WIP] Update ini package to 1.3.8 & merge-deep to Update ini package to 1.3.8 & merge-deep to Jun 3, 2021
@botify
Copy link

botify commented Jun 3, 2021

npm has a package.json file and a package-lock.json file. It seems you updated one without the other, which is usually a sign of a mistake. If you are updating a package make sure that you update the version in package.json then run npm install

@deetergp
Copy link
Contributor Author

deetergp commented Jun 3, 2021

Alright, I've tested, removed the WIP, and calling it ready for review.

@jasperhuangg jasperhuangg merged commit 734411e into main Jun 4, 2021
@jasperhuangg jasperhuangg deleted the scott-fixSNYK-JS-INI-1048974 branch June 4, 2021 05:58
@OSBotify
Copy link
Contributor

OSBotify commented Jun 4, 2021

🚀 Deployed to staging in version: 1.0.62-3🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

@OSBotify
Copy link
Contributor

OSBotify commented Jun 8, 2021

🚀 Deployed to production in version: 1.0.64-0🚀

platform result
🤖 android 🤖 failure ❌
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

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

Successfully merging this pull request may close these issues.

4 participants