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.

Sign up for GitHub

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

closurebuilder - Allow any value for MONETATE_FLAG #2

Merged
merged 1 commit into from
May 2, 2018
Merged

closurebuilder - Allow any value for MONETATE_FLAG #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions closure/bin/build/closurebuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
^(mc\. # Must be a property of the monetate common namespace. Capture group 1.
[a-zA-Z]+\. # Must have it's own module namespace. Capture group 1.
[A-Z0-9_]+) # Must follow the constant format. Capture group 1.
=(true|false)$ # Must assign a boolean. Capture group 2.
=(.*)$ # Must assign a value. Capture group 2.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capturing anything seems a bit "dangerous" since now there will be no true validation on the value of the feature flag. Could potentially put values in there that aren't valid for the flag being set and nothing will be spit out as an error.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if we add possible values here, it can still fail because for example a variable that's a number gets overriden with a boolean or viceversa. I agree with your concern, but not sure if there's a good way to solve this.


"""

Expand Down Expand Up @@ -277,7 +277,7 @@ def main():
monetate_flag_value = monetate_flag_group[1]

# Make sure property is defined in the source.
monetate_define_re = re.compile('%s\s*=\s*(true|false)\s*;' % monetate_flag_property)
monetate_define_re = re.compile('%s\s*=\s*(.*)\s*;' % monetate_flag_property)
if not monetate_define_re.search(script_source):
logging.error('--monetate-flag: %s was not found in script source.', monetate_flag_property)
sys.exit(2)
Expand Down