-
Notifications
You must be signed in to change notification settings - Fork 13
Extend --link flag to support explicit kinds #1128
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
Conversation
This comment has been minimized.
This comment has been minimized.
|
I'll review this PR by this weekend. |
|
Can you add the prefix e.g. https://github.com/cloudbees-oss/smart-tests-cli/blob/main/launchable/testpath.py#L46 |
launchable/utils/link.py
Outdated
| for k, url in link_options: | ||
| # if k,v in format "kind|title=url" | ||
| if '|' in k: | ||
| kind, title = k.split('|', 1) |
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.
Should we handle the whitespace?
e.g. GITHUB_PULL_REQUEST | PR = https://github.com/launchableinc/cli/pull/2
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.
I don't have a strong opinion on this. I stripped the values to be forgiving to user error.
| CIRCLECI_BUILD_NUM_KEY = 'CIRCLE_BUILD_NUM' | ||
| CIRCLECI_JOB_KEY = 'CIRCLE_JOB' | ||
|
|
||
| GITHUB_PR_REGEX = re.compile(r"^https://github\.com/[^/]+/[^/]+/pull/\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.
Can we use the named group? Here is the example:
>>> p = re.compile(r'(?P<word>\b\w+\b)')
>>> m = p.search( '(((( Lots of punctuation )))' )
>>> m.group('word')
'Lots'
>>> m.group(1)
'Lots'
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.
Since we are not extracting any parts of the URL, I think it's better to keep it simple.
This PR extends
--linkoption to support both the formats"kind|title=url"and"title=url"Example usages:
Custom links:
Invalid kind:
Invalid URL for kind:
Infer kind:
GitHub PR links are automatically inferred as GITHUB_PULL_REQUEST.
Prioritize link options over env links:
Here GITHUB_PULL_REQUEST URL passed in the link option will be used instead of the URL set via the environment variable.