smapiot / piral

🚀 Framework for next generation web apps using micro frontends. ⭐️ Star to support our work!
https://piral.io
MIT License
1.71k stars 127 forks source link

Feature/#546 add app name argument on new piral #551

Closed mgarciaar closed 2 years ago

mgarciaar commented 2 years ago

New Pull Request

For more information, see the CONTRIBUTING guide.

Prerequisites

Please make sure you can check the following boxes:

Type(s) of Changes

Contribution Type

What types of changes does your code introduce? Put an x in all the boxes that apply:

Description

I added a --appName property to the piral new command in order to define the package name without having to edit the package.json manually afterwards.

Remarks

I've added a simple test that checks the package.json "name" content. As there are other tests checking for the scaffolding structure I decided to maintain those checks.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

github-actions[bot] commented 2 years ago

File size impact

Merging feature/#546-add-appName-argument-on-new-piral into develop impact files as follow:

minimal-piral (no impact)
Files new size
Unmodified (2) 250 kB (0 B / +0%) :ghost:
Total (2) 250 kB (0 B / +0%) :ghost:
sample-cross-fx (no impact)
Files new size
Unmodified (3) 2.04 MB (0 B / +0%) :ghost:
Total (3) 2.04 MB (0 B / +0%) :ghost:
sample-piral (no impact)
Files new size
Unmodified (3) 636 kB (0 B / +0%) :ghost:
Total (3) 636 kB (0 B / +0%) :ghost:
sample-piral-core (no impact)
Files new size
Unmodified (3) 304 kB (0 B / +0%) :ghost:
Total (3) 304 kB (0 B / +0%) :ghost:

Generated by @jsenv/file-size-impact during check-bundle-size#3169819349 on 921cfc3

mgarciaar commented 2 years ago

Yeah looks great - only thing I'd change for consistency is to use --app-name instead of --appName. Reason is that we also use, e.g., --force-overwrite and --npm-client instead of --forceOverwrite and --npmClient. This way, one always knows that camelCase (e.g., appName) is used within JS and dash-case (app-name) is for for CLI.

Ok, looks good to me. I'll change it now.