-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[SYNTH-18711] Update comment about sed syntax support #28482
[SYNTH-18711] Update comment about sed syntax support #28482
Conversation
Preview links (active after the
|
636aba0
to
d18f7fc
Compare
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.
LGTM
<div class="alert alert-info"> | ||
Apart from the pipe <code>|</code> syntax presented above, <code>resourceUrlSubstitutionRegexes</code> also supports the sed syntax with modifiers: <code>s|<regex>|<rewriting rule>|<modifiers></code>.</br></br> The sed syntax is often used with a slash <code>/</code> separator, for example: <code>s/<regex>/<rewriting rule>/<modifier></code>. However, it can use any character as a delimiter. When working on a URL containing an abundant number of slashes, Datadog recommends using another character rather than escaping all slashes of the URL. | ||
Only the pipe <code>|</code> syntax is supported for URL substitution regexes. Sed syntax and other delimiter characters are not supported. | ||
</div> | ||
|
||
## Further reading |
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.
Not you but the what's next
thing is broken and we should tell someone :D
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.
@teodor2312 Can you tell me more about what's broken?
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.
We do support two syntaxes
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.
LGTM, let's see what @urseberry thinks about the wording added in 86652ed
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
What does this PR do? What is the motivation?
This PR fixes incorrect documentation about URL substitution syntax in Continuous Testing. The documentation previously claimed support for sed syntax and different delimiters, which was incorrect. The changes remove this misleading information.
Merge instructions
Merge readiness:
For Datadog employees:
Merge queue is enabled in this repo. Your branch name MUST follow the
<name>/<description>
convention and include the forward slash (/
). Without this format, your pull request will not pass in CI, the GitLab pipeline will not run, and you won't get a branch preview. Getting a branch preview makes it easier for us to check any issues with your PR, such as broken links.If your branch doesn't follow this format, rename it or create a new branch and PR.
To have your PR automatically merged after it receives the required reviews, add the following PR comment:
Additional notes