-
-
Notifications
You must be signed in to change notification settings - Fork 816
Update babel-eslint to 8.1.1 #2255
Conversation
Signed-off-by: Aaron Raimist <[email protected]>
|
Surely this requires a change to the package lock too? |
|
It... should? but nothing changed in it |
|
weiiird. Alright then. assuming the build passes this lgtm |
|
Actually I know why nothing changed 🤦♂️. |
|
Oh, that would do it too. |
|
Looks like upgrading eslint caused lint failures |
|
Updating to
brings it down to 10 errors (which actually look reasonable) -> sleep and I'll fix this up tomorrow |
Signed-off-by: Aaron Raimist <[email protected]>
Signed-off-by: Aaron Raimist <[email protected]>
Signed-off-by: Aaron Raimist <[email protected]>
|
Never mind I'll just fix it now. When reviewing this pay special attention to 5f3b03c. I think this is right but double check. |
Signed-off-by: Aaron Raimist <[email protected]>
|
Sorry it is so massive, I don't think there's a way to |
| if (!this.state.groupId) { | ||
| error = _t("Community IDs cannot be empty."); | ||
| } else if (!/^[a-z0-9=_\-\.\/]*$/.test(this.state.groupId)) { | ||
| } else if (!/^[a-z0-9=_\-./]*$/.test(this.state.groupId)) { |
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.
This doesn't seem right
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.
Current version
\. matches the character . literally (case sensitive)
\/ matches the character / literally (case sensitive)
Proposed version:
./ matches a single character in the list ./ (case sensitive)
Same thing
| // For URLs of matrix.to links in the timeline which have been reformatted by | ||
| // HttpUtils transformTags to relative links. This excludes event URLs (with `[^\/]*`) | ||
| const REGEX_LOCAL_MATRIXTO = /^#\/(?:user|room|group)\/(([#!@+])[^\/]*)$/; | ||
| const REGEX_LOCAL_MATRIXTO = /^#\/(?:user|room|group)\/(([#!@+])[^/]*)$/; |
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.
This also doesn't seem right... I think...
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.
According to https://regex101.com/r/1M2H7B/2/
\/ matches the character / literally (case sensitive)
and
/ matches the character / literally (case sensitive)
so they are the same
| */ | ||
|
|
||
| const PHONE_NUMBER_REGEXP = /^[0-9 -\.]+$/; | ||
| const PHONE_NUMBER_REGEXP = /^[0-9 -.]+$/; |
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.
Also seems a bit weird... maybe
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.
These also seem to be the same https://regex101.com/r/1M2H7B/3
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.
Thank you!
|
Thanks for the PR, although just a couple of notes since I just got slightly surprised by this: I'd been writing |
Required for element-hq/element-web#7490