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.

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

RUMM-700 Environment regex changed #246

Merged
merged 1 commit into from Sep 10, 2020

Conversation

buranmert
Copy link
Contributor

What and why?

There was a discrepancy between Android's environment validation rule and ours.
For instance, shop.ist was invalid for us whereas it was valid for Android and more importantly valid for RUM Explorer.

How?

Now we use Android's regex, which allows "_./-" as non-alphanumerics.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@buranmert buranmert added this to the rum milestone Sep 9, 2020
@buranmert buranmert requested a review from a team as a code owner September 9, 2020 16:54
@buranmert buranmert self-assigned this Sep 9, 2020
Copy link
Collaborator

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

The JIRA also says to update the env in Shopist project to match the one used in Android.

@@ -140,9 +140,9 @@ extension FeaturesConfiguration {
}

private func ifValid(environment: String) throws -> String {
let regex = #"^[a-zA-Z0-9_]+$"#
let regex = #"^[a-zA-Z0-9_./-]+$"#
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are more rules to include in this regexp, i.e. "must start with a letter", "can be up to 200 characters long".

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ref to Android regexp: DataDog/dd-sdk-android#369 (comment)

Copy link
Collaborator

@ncreated ncreated left a comment

Choose a reason for hiding this comment

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

  • The JIRA also says to update the env in Shopist project to match the one used in Android.

Sources/Datadog/Core/FeaturesConfiguration.swift Outdated Show resolved Hide resolved
@buranmert buranmert force-pushed the buranmert/RUMM-700-env-validation branch from 0e9eea7 to 4b37e93 Compare September 10, 2020 10:06
Android's regex is used,
which allows "_./-" as non-alphanumerics
@buranmert buranmert merged commit 3367c56 into master Sep 10, 2020
@buranmert buranmert deleted the buranmert/RUMM-700-env-validation branch September 10, 2020 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants