-
Notifications
You must be signed in to change notification settings - Fork 905
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
fix: be able to read more complex buildToolsVersion definitions #2531
Conversation
@cipolleschi friendly bump on this one ;) Any feedback? |
@@ -50,8 +50,8 @@ const getBuildToolsVersion = (projectRoot = ''): string => { | |||
.substring(buildToolsVersionIndex) | |||
.split('\n')[0] | |||
// Get only the the value of `buildToolsVersion` | |||
.match(/\d|\../g) || [] | |||
).join(''); | |||
.match(/\d+/g) || [] |
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.
if the version is always three parts, shouldn't we do /\d+\.\d+\.\d+/g
? 🤔
This probably allow to drop the join
below 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.
Good catch. Build tools versions are \d+\.\d+\.\d+
. PR updated.
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.
If it works, it looks ok to me. I suggested a more precise approach, but I'm not super familiar on how the versioning of the Android build tool is defined.
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 a huge fan of us doing those Regex magic but I guess that's at least an improvement from our status quo
Thank you! |
Summary:
As highlighted in #2221, the way some projects (like expo ones) define the
buildToolsVersion
is breakingreact-native doctor
.leads to
buildToolsVersion => .b34.0.0
.The PR is slightly tweaking the parsing to account for those cases.
Test Plan:
buildToolsVersion
buildToolsVersion = "a version"
syntaxChecklist
[ ] Documentation is up to date to reflect these changes.NA