Create CLI questionnaire #57

macklinu · 2019-02-28T05:45:42Z

Please squash and merge this PR when ready to avoid merging so many intermediate commits. ❤️

Why?

Resolves #54, and because it's fun to improve developer experience. 😄

How?

bin/cli.js is now the CLI entrypoint. It requires bin/create-action.js, which is a Promise that parses CLI arguments, reads files, writes files, and prompts the user for information to fill out the required metadata for their GitHub Action.

bin/create-actions.js has been split out into its own file so that the Promise function is unit testable on its own.

This PR also includes the scripts/update-feather-icons.js, which pulls a list of feather icon names that can be selected during the CLI prompt.

I suggest pulling down this branch and playing around with the new CLI experience - feedback is definitely welcome on UI/messaging in the CLI.

Test scenarios:

Locally, I have been running node path/to/actions-toolkit-repo/bin/cli.js, which is essentially what npx actions-toolkit will run for users of this CLI.

npx actions-toolkit without args should display help message
npx actions-toolkit --help should display help message
npx actions-toolkit <directory-name> should start the CLI prompt to ask for name, description, etc.
- finish the prompt flow and ensure a new actions-toolkit directory is bootstrapped, and that you can npm install it successfully
- also, try canceling during that prompt to see a cancellation message (not sure if necessary)

Tests have been added/updated (if necessary)
Documentation has been updated (if necessary)

bin/feather-icons.json

bin/actions-toolkit.js

bin/template/Dockerfile

tests/cli.test.js

__mocks__/fs.js

bin/template/Dockerfile

JasonEtco · 2019-02-28T06:10:12Z

This is lookin' great so far @macklinu 😍 also the first time I've seen a draft PR from a fork, I dig it. Thanks for doing this and for the super helpful hints above ❤️

scripts/update-feather-icons.js

macklinu

Alright @JasonEtco, getting much closer to a final review. I think one more review of the architecture and tests here would be helpful to make sure I'm still going in the right direction. If anything is confusing, please comment and I can explain further (both with PR comments and perhaps some code comments to help explain some of the mocking stuff, if that'd help).

Based on this PR's feedback, I should be able to finish this up likely early next week. Thanks! 🎨

bin/actions-toolkit.js

tests/__mocks__/fs.js

tests/cli.test.js

macklinu · 2019-03-06T18:32:13Z

I'm about one pass away from requesting a full review. Should be able to take another look today or tomorrow. 👍

macklinu

@JasonEtco this is ready for review! 😸 Left some inline comments, and I also updated the PR description to further explain my decisions and some manual test cases I've run to test this feature locally.

macklinu · 2019-03-08T01:11:42Z

bin/cli.js

+
+const createAction = require('./create-action')
+
+createAction(process.argv.slice(2))


This is now the main CLI entrypoint - also defined in package.json.

bin/create-action.js

tests/create-action.test.js

bin/create-action.js

JasonEtco · 2019-03-10T04:01:22Z

This is amazing @macklinu. Thanks for the time and effort you've put into this work ❤️ I did some light reviewing, but I'll dedicate some time soon for a full review - hopefully we'll be able to wrap this up and get it merged next week 🤞

bin/template/Dockerfile

Co-Authored-By: macklinu <[email protected]>

Using Promise.all is better than Array.prototype.forEach for the purposes of writing multiple files to disk.

macklinu · 2019-03-14T18:47:45Z

@JasonEtco rebased against master and pushed up a commit to address the most recent PR comments. 👍

bin/create-action.js

JasonEtco · 2019-03-16T21:37:31Z

Ok so I've given this a real review @macklinu - its really solid, the experience is 👌. The one thing I want to improve is the logging - we're using console.log and it looks kinda funny next to Enquirer's fancier, bold/green logs. Fortunately, we have Signale already available to us. Here's what it looks like in 9bbf52d:

Unfortunately, this totally borks in tests - I've tracked it down to be caused by the mock fs, since Signale tries to use it internally. I'm actually happy this came up, because I'm not sure that mocking fs is a good thing anyway - it means we're relying on our mocked logic instead of how fs really works. So, I think we should update the tests to instead read/write to/from real files in a fixtures folder. It'll make for more realistic tests, despite being a little more effort to delete/create the files as needed.

JasonEtco · 2019-03-22T20:31:04Z

@macklinu I'd love to get this into v2.0.0 in #62 - I can take over the changes if you'd like, or you can carry it over the finish line. I'm really hoping to publish v2 this upcoming weekend 🤞

macklinu · 2019-03-22T20:45:49Z

I’ll follow up on this today - thanks!

tests/create-action.test.js

macklinu · 2019-03-23T20:38:46Z

tests/__snapshots__/create-action.test.js.snap

+
+exports[`creates project with labels passed to Dockerfile from questionnaire: package.json 1`] = `
+{
+  "name": "__tmp/my-project-name",


Not sure this is desired and might be an implementation bug? Check out this test - essentially it runs:

npx actions-toolkit __tmp/my-project-name

Which would create a folder a couple of levels deep. Might be a bug or perhaps a side-effect of my test implementation and would love another set of eyes on this. 👀 ❤️

Interesting... so when I try running that, it gives me a "__tmp doesn't exist" when it tries to make the my-project-name dir (which makes sense).

That said, I think its fine? Thanks for calling it out 🙏

JasonEtco

From my perspective, this is good to 🚢 thanks a million for your hard work @macklinu - seriously, this is a great enhancement and took a lot of effort ❤️ Because of the complexity of the PR, I'll hold off until you give me a 👍 to merge, just to make sure there's nothing else you're looking to do. 🎉

macklinu · 2019-03-23T22:29:41Z

Nothing else I’m looking to do here - thanks for your help in collaborating on this feature with me! 😀👏

macklinu commented Feb 28, 2019

View reviewed changes

JasonEtco reviewed Mar 1, 2019

View reviewed changes

scripts/update-feather-icons.js Outdated Show resolved Hide resolved

macklinu commented Mar 1, 2019

View reviewed changes

bin/actions-toolkit.js Outdated Show resolved Hide resolved

tests/__mocks__/fs.js Outdated Show resolved Hide resolved

tests/cli.test.js Outdated Show resolved Hide resolved

tests/cli.test.js Outdated Show resolved Hide resolved

macklinu marked this pull request as ready for review March 8, 2019 00:52

macklinu changed the title ~~WIP: Create CLI questionnaire~~ Create CLI questionnaire Mar 8, 2019

macklinu commented Mar 8, 2019

View reviewed changes

bin/create-action.js Outdated Show resolved Hide resolved

macklinu mentioned this pull request Mar 10, 2019

Update to Jest 24.3.x #58

Merged

2 tasks

JasonEtco reviewed Mar 14, 2019

View reviewed changes

bin/template/Dockerfile Outdated Show resolved Hide resolved

macklinu and others added 12 commits March 14, 2019 11:42

WIP: Create CLI questionnaire

2c4ed54

Replace mustache with custom solution

456cdeb

Add scripts/update-feather-icons.js

78012c5

Use Map.prototype.clear in fs mock

6b92afe

Get tests passing :D

d319112

Update scripts/update-feather-icons.js

da14ce9

Co-Authored-By: macklinu <[email protected]>

Update tests/cli.test.js

dde2810

Co-Authored-By: macklinu <[email protected]>

More updates / WIP

db2c9cb

More documenatation

9232edb

Tweak welcome message spacing

9a02778

Fix async write file loop

27725dd

Using Promise.all is better than Array.prototype.forEach for the purposes of writing multiple files to disk.

Address PR comments

fc3dd0b

macklinu force-pushed the cli-questionnaire branch from 97e1417 to fc3dd0b Compare March 14, 2019 18:46

macklinu commented Mar 14, 2019

View reviewed changes

bin/create-action.js Outdated Show resolved Hide resolved

macklinu and others added 3 commits March 14, 2019 12:14

Add and update tests for 100% test coverage

66c812d

Merge branch 'master' into cli-questionnaire

863619f

Couple small tweaks

ee68111

JasonEtco added 2 commits March 16, 2019 17:13

Whitespace and comments

5ba626d

Use Signale

9bbf52d

macklinu commented Mar 23, 2019

View reviewed changes

tests/create-action.test.js Show resolved Hide resolved

macklinu added 2 commits March 23, 2019 13:09

Update tests to use real filesystem

b89f13a

Merge remote-tracking branch 'upstream/master' into cli-questionnaire

aa828d6

macklinu commented Mar 23, 2019

View reviewed changes

JasonEtco added 2 commits March 23, 2019 16:53

Fix usage log

8bbe9a0

Fix test

3a4b2dc

JasonEtco approved these changes Mar 23, 2019

View reviewed changes

JasonEtco merged commit 9919555 into JasonEtco:master Mar 23, 2019

macklinu mentioned this pull request Mar 29, 2019

Not all feather icons are supported by actions #68

Closed


		const createAction = require('./create-action')

		createAction(process.argv.slice(2))

Create CLI questionnaire #57

Create CLI questionnaire #57

Uh oh!

Conversation

macklinu commented Feb 28, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JasonEtco commented Feb 28, 2019

Uh oh!

Uh oh!

macklinu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

macklinu commented Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macklinu left a comment

Choose a reason for hiding this comment

Uh oh!

macklinu Mar 8, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JasonEtco commented Mar 10, 2019

Uh oh!

Uh oh!

macklinu commented Mar 14, 2019

Uh oh!

Uh oh!

JasonEtco commented Mar 16, 2019

Uh oh!

JasonEtco commented Mar 22, 2019

Uh oh!

macklinu commented Mar 22, 2019 via email • edited by JasonEtco Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

macklinu Mar 23, 2019

Choose a reason for hiding this comment

Uh oh!

JasonEtco Mar 23, 2019

Choose a reason for hiding this comment

Uh oh!

JasonEtco left a comment

Choose a reason for hiding this comment

Uh oh!

macklinu commented Mar 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

macklinu commented Feb 28, 2019 •

edited

Loading

macklinu commented Mar 6, 2019 •

edited

Loading

macklinu commented Mar 22, 2019 via email •

edited by JasonEtco

Loading