Skip to content
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

CLI: Add install command #18504

Merged
merged 74 commits into from
Feb 4, 2021
Merged

CLI: Add install command #18504

merged 74 commits into from
Feb 4, 2021

Conversation

kraftbj
Copy link
Contributor

@kraftbj kraftbj commented Jan 22, 2021

Changes proposed in this Pull Request:

  • Adds jetpack install which runs composer install and yarn install if composer.json or package.json are present in a project.

Also, prettifies the build step:

Screen.Recording.2021-01-22.at.5.26.01.PM.mov

Jetpack product discussion

n/a

Does this pull request change what data or activity we track or use?

n/a

Testing instructions:

  • yarn install for the initial install.
  • Either run yarn cli-link or prefix all of the commands below with .tools/cli/bin/, e.g. ./tools/cli/bin/jetpack install --help.
  • jetpack to see all help commands.
  • jetpack install --help to see the install commands.
  • jetpack install to get a project picker.
  • jetpack install --root to get a project picker and install the monorepo root too.
  • jetpack install monorepo to install the monorepo root without a project.
  • jetpack install packages/connection to install that particular package.
  • jetpack install packages/connection --root to install a particular package and the repo root.
  • jetpack build plugins/jetpack to see the new build prettification.

Proposed changelog entry for your changes:

Screen.Recording.2021-01-22.at.3.19.34.PM.mov

@kraftbj
Copy link
Contributor Author

kraftbj commented Feb 3, 2021

@anomiex I want to DRY out the command a bit, but end of my day so wanted to push up to get a gut check that this makes you happy wrt the github action providing verbose output. https://github.com/Automattic/jetpack/pull/18504/checks?check_run_id=1826705537
2021-02-03 at 5 37 PM

@anomiex
Copy link
Contributor

anomiex commented Feb 4, 2021

Haven't checked the code yet, but the verbose output looks fine to me.

If I were writing an action-specific tool I'd put a ::group::Installing $SLUG before the [23:35:53] Installing $SLUG [started] line and ::endgroup:: after the corresponding [23:36:06] Installing $SLUG [completed], so the GH Actions log would group and collapse each tool's install commands. Which, if you really want to do, you could look for an appropriate environment variable to trigger that. But unless it's super easy to do it I wouldn't bother since this isn't an action-specific tool, and at least there is clear grouping even if GH doesn't pick up on it.

projects/github-actions/.gitignore Outdated Show resolved Hide resolved
tools/cli/commands/install.js Show resolved Hide resolved
tools/cli/helpers/normalizeArgv.js Outdated Show resolved Hide resolved
sdixon194
sdixon194 previously approved these changes Feb 4, 2021
@kraftbj
Copy link
Contributor Author

kraftbj commented Feb 4, 2021

Going to land this. If there are more issues, this is under active iteration and we'll keep with it.

sdixon194
sdixon194 previously approved these changes Feb 4, 2021
.gitignore Outdated Show resolved Hide resolved
Comment on lines 114 to 116
* @param {object} tasks - The tasks object
*
* @returns {object} An array of either the root install task or empty.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param {object} tasks - The tasks object
*
* @returns {object} An array of either the root install task or empty.
* @param {Array} tasks - The tasks array
*
* @returns {Array} The tasks array, which may have been modified.

tools/cli/commands/install.js Outdated Show resolved Hide resolved
Co-authored-by: Brad Jorsch <anomiex@users.noreply.github.com>
@kraftbj kraftbj merged commit 677b884 into master Feb 4, 2021
@kraftbj kraftbj deleted the add/cli-install branch February 4, 2021 20:33
@matticbot matticbot added [Status] Needs Changelog and removed [Status] Needs Review To request a review from Crew. Label will be renamed soon. labels Feb 4, 2021
anomiex added a commit that referenced this pull request Mar 5, 2021
PR #18504 increased the limit for one job that was consistently hitting
the old 5-minute timeout, but not the other two. Lately I've been
noticing that one of those other two has been barely hitting the timeout
too on occasion, and I see the other is at ~4:30 too, so let's bump them
too.
anomiex added a commit that referenced this pull request Mar 5, 2021
PR #18504 increased the limit for one job that was consistently hitting
the old 5-minute timeout, but not the other two. Lately I've been
noticing that one of those other two has been barely hitting the timeout
too on occasion, and I see the other is at ~4:30 too, so let's bump them
too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Needs Package Release This PR made changes to a package. Let's update that package now. [Tools] Development CLI The tools/cli to assist during JP development. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants