This repository was archived by the owner on Nov 6, 2023. It is now read-only.
Repository navigation
Conversation
cschanaj
changed the title
cschanaj
changed the title
cschanaj
marked this pull request as ready for review
April 13, 2021 01:46
Hainish
reviewed
Apr 13, 2021
| settings.enableMixedRulesets, | ||
| this.ruleActiveStates, | ||
| scope); | ||
| getScope(scope)); |
Member
There was a problem hiding this comment.
Why change this string to a scope in JavaScript? The scope will be cached when passed to Rust at any rate. It looks like this passes a RegExp to the Rust bindings, which it isn't expecting.
Collaborator
Author
There was a problem hiding this comment.
Look like a mistake made when I was trying to figure out the types of scope in different calls. The original source code uses string scope too. It should be a string. Changes made in f87434f
As the getScope function is being called in Ruleset constructor exclusively, I added 1c20e82 to reflect this. Only the internal representation of the scope is a RegExp now. Thanks.
Member
|
LGTM, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR makes compiling
scopeintoRegExpautomatic. Pass the string literal version of thescopeparameter everywhere, except for when constructing rulesets/WASM calls.getScopeexpected a required string parameter, but get none. It is also called multiple time when passed toaddFromJsonType of change