-
Notifications
You must be signed in to change notification settings - Fork 943
Allow leading zeros in tryParseInt #2212
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
Conversation
Co-Authored-By: Jeffrey Dallatezza <[email protected]>
packages/database/test/path.test.ts
Outdated
@@ -20,8 +20,8 @@ import { Path } from '../src/core/util/Path'; | |||
|
|||
describe('Path Tests', function() { | |||
const expectGreater = function(left, right) { | |||
expect(Path.comparePaths(new Path(left), new Path(right))).to.equal(1); | |||
expect(Path.comparePaths(new Path(right), new Path(left))).to.equal(-1); | |||
expect(Path.comparePaths(new Path(left), new Path(right))).to.be.greaterThan(0) |
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.
missing semicolon?
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.
Well, on GitHub it is missing. But I swear that I have this locally (via the Prettier commit that once again got stuck somewhere...)
…-js-sdk into mrschmidt/leadingzeros
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.
FWIW (this isn't assigned to anybody so maybe you don't care if it's reviewed)- LGTM assuming CI being completely broken is unrelated to your changes.
CI is passing now after a re-run. |
Fixes https://buganizer.corp.google.com/issues/141298281
A customer noticed that we don't correctly order their documents. Some of their documents are using keys of the form "00x", where x is a 9 digit number. Our number check only allows for ten digits, and leading zeros can break this limit. This PR relaxes the check and matches the backend semantics.