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.
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
Add android sdk module #9236
Add android sdk module #9236
Changes from 43 commits
215467f
2526420
ec674cd
ca3d11a
7349127
01c3674
d7f6451
0434249
3edfac2
ba7f1bf
de6f740
97b07ee
b41eb57
6d756e2
4441be7
100a91f
fdef9c4
9b3d444
ece03d1
e8ef594
1ac1ad5
22b317f
e499391
7e53827
e5a6d02
003bd1e
20eb1aa
3358ac9
14f0d6e
1ad1355
eb10db3
7e1af46
baa5f65
d367bd2
619f28d
751ec9e
2089a5b
8008062
165f04e
c5474f2
83dd52e
ffd170d
bf10fe0
1878cd0
043fc16
7be3b3c
9033695
2159c01
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Well, from these examples, the only two possibilities that are not covered by the suggested regexp (the ones with
rc1
andrc2
) are also not covered by the existing regexp:|\s*\S+\s*\|
will not match| 32.1.0 rc1 |
(because the\S+
will not match the whitespace beforerc1
).The one thing that is the delimiter here is the
|
character, so maybe it would be better of as:That forces the version to begin with a digit, then anything-but-the-vertical-bar until the space and vertical bar. The
\b
is used to make sure the "anything-but-the-vertical-bar" part does not include the whitespaces before the bar.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.
As to your question of whether it makes sense to cover all the cases with the regexp, I would answer that IMHO it makes more sense to cover all the search cases with a search tool (regexp) then to cover only part of the search with it and create another search mechanism later on. Just my $0.02 ;-)
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.
Good catch, thanks
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.
You would need to change this regexp to search for numbers as well.
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.
See comment above