Skip to content

kool create command#198

Merged
fabriciojs merged 14 commits intokool-dev:masterfrom
leandroncbrito:create-command
Nov 25, 2020
Merged

kool create command#198
fabriciojs merged 14 commits intokool-dev:masterfrom
leandroncbrito:create-command

Conversation

@leandroncbrito
Copy link
Copy Markdown
Contributor

#17

New command "create"

Parameters:

  1. preset/framework name
  2. project/folder name

Ex: kool create laravel my-app

New issue improvement:
Instead of entering the preset name, we might show the preset menu and let the user choose.

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 16, 2020

Codecov Report

Merging #198 (01c5fbe) into master (805f616) will decrease coverage by 0.27%.
The diff coverage is 81.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #198      +/-   ##
==========================================
- Coverage   89.22%   88.94%   -0.28%     
==========================================
  Files          52       53       +1     
  Lines        1652     1710      +58     
==========================================
+ Hits         1474     1521      +47     
- Misses        152      160       +8     
- Partials       26       29       +3     
Impacted Files Coverage Δ
cmd/builder/fake_command.go 84.21% <0.00%> (-15.79%) ⬇️
cmd/presets/fake_parser.go 90.47% <60.00%> (-9.53%) ⬇️
cmd/create.go 81.81% <81.81%> (ø)
cmd/builder/command.go 91.30% <100.00%> (+1.30%) ⬆️
cmd/preset.go 100.00% <100.00%> (ø)
cmd/presets/parser.go 48.07% <100.00%> (+5.52%) ⬆️
cmd/presets/presets.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 805f616...74ef333. Read the comment docs.

@dbpolito
Copy link
Copy Markdown
Member

@leandroncbrito after you create, it's already having the kool files there? or you still need to init/preset afterwards?

I think we can safely create and init.

Comment thread cmd/create.go Outdated
Comment thread cmd/create.go Outdated
Comment thread cmd/presets/fake_parser.go
@danielsuguimoto
Copy link
Copy Markdown
Contributor

@leandroncbrito after you create, it's already having the kool files there? or you still need to init/preset afterwards?

I think we can safely create and init.

I like the idea too. The only question I have: should we add an option for creating the project only, without copying the preset files?

@dbpolito
Copy link
Copy Markdown
Member

Yeah but i think it makes sense creating it as default... maybe a --no-preset flag to disable it?

@fabriciojs
Copy link
Copy Markdown
Member

Yeah but i think it makes sense creating it as default... maybe a --no-preset flag to disable it?

No need for this flag IMO. Design wise, we want people to use our environment. Even if they do not want to, having the files installed is not a big deal - no issues should arise from that.

@leandroncbrito
Copy link
Copy Markdown
Contributor Author

Yeah, I like the idea.
I'll make the code improvements that Jun left and take a look at this change.

Copy link
Copy Markdown
Contributor

@danielsuguimoto danielsuguimoto left a comment

Choose a reason for hiding this comment

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

Nice! I would add just one test case to the create command. If we add a test case and mock the preset fake parser to return true on the method Exists (set MockExists to true), then we can improve the test coverage. This way I guess all checks for this PR will pass.

https://codecov.io/gh/kool-dev/kool/src/create-command/cmd/create.go
image

Copy link
Copy Markdown
Contributor

@danielsuguimoto danielsuguimoto left a comment

Choose a reason for hiding this comment

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

LGTM! Thx!

Copy link
Copy Markdown
Contributor

@danielsuguimoto danielsuguimoto left a comment

Choose a reason for hiding this comment

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

@leandroncbrito sorry, I was almost forgetting about the docs. Please, run kool run make-docs to generate the docs markdown to this new command, then I think it's ready to go.

@leandroncbrito
Copy link
Copy Markdown
Contributor Author

@leandroncbrito sorry, I was almost forgetting about the docs. Please, run kool run make-docs to generate the docs markdown to this new command, then I think it's ready to go.

done

Comment thread docs/4-Commands/0-kool.md
Comment thread docs/4-Commands/kool-init.md
@danielsuguimoto
Copy link
Copy Markdown
Contributor

LGTM! What do you think @dbpolito @fabriciojs ?

Copy link
Copy Markdown
Member

@fabriciojs fabriciojs left a comment

Choose a reason for hiding this comment

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

@leandroncbrito looking great! thanks for all the work on this!

I left a handful of comments, some we might leave for a feature iteration, others we may discuss further. Let me know what you think!

Comment thread cmd/presets/parser.go Outdated
Comment thread cmd/create.go Outdated
Comment thread cmd/create.go Outdated
Comment thread cmd/create.go
Comment thread cmd/presets/parser.go
Comment thread cmd/presets/presets.go Outdated
@fabriciojs fabriciojs merged commit fd71b81 into kool-dev:master Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants