-
Notifications
You must be signed in to change notification settings - Fork 373
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
feat(firestore): Version update @google-cloud/firestore to 5.x #1525
feat(firestore): Version update @google-cloud/firestore to 5.x #1525
Conversation
Sorry, I didn't sign the CLA, I signed it. |
Hi @makibishi0212 Thank you for the contribution! Were you able to build and run unit tests locally for this change? It looks like our CIs are failing. |
It also seems Firestore 5.0 has breaking changes. Might want to be careful with our own semver here. |
@lahirumaramba Thank you for review this PR!
I checked unit test(by |
@lahirumaramba |
I'm sorry for repeating myself. Updated |
@makibishi0212 Thank you for updating the PR! It appears that a change introduced to tuples in TS 4 breaks the version of I suggest we update Let's keep this PR for its original purpose, which is to update Also, it seems like the updates to
To fix this, you could try and sign the CLA from the second account as well, or try to change the author of the last commit:
Thank you again for your contribution! Let me know if you have any questions. |
@lahirumaramba Thanks for the review.
I understand about the phased update here. Indeed, this change was an ad-hoc fix to an error in eslint, which is not a good change. After the And I will also address the author issue. |
@makibishi0212 The ESLint #1540 and TS #1541 updates are now merged. Please rebase this PR when you get a chance. |
82e16d9
to
03c9f46
Compare
03c9f46
to
165c229
Compare
@lahirumaramba Thanks for the ESLint and TS updates! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM!
This PR resolves #1524
An update to typescript is also included in the PR to resolve the compilation error that occurs in Google-cloud/firestore.