-
Notifications
You must be signed in to change notification settings - Fork 67
Introduce Version Tool into Build Tools. #331
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
base: 2.x
Are you sure you want to change the base?
Conversation
|
@rvtraveller made some changes - @greg-1-anderson could you review if this is good to be merged now? Thank you! |
| $application = $info->application(); | ||
| $version = $info->version(); | ||
| $major_version = explode(".", $version); | ||
| $framework = $application == "WordPress" ? "empty-wordpress" : ( |
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.
This could be a little more verbose for readability. Maybe break it out into its own function where you can use the pattern if (cond) { return } to avoid nesting and ternary operators.
| $data = [ | ||
| 'application' => $info['application'], | ||
| 'version' => $info['version'], | ||
| 'major-version' => $info['major_version'], |
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 we used $info['major-version'] instead of major_version throughout the code, then we could just return $info here without having to create a mapping like this.
Let's introduce the Version Tool package into Build Tools so we can leverage it in other PRs or work we're doing.
This includes a
build:project:versioncommand which is largely available just for testing purposes.