Add gulp option #22
Add gulp option #22
Conversation
There was a problem hiding this comment.
Dobrý den Jendo,
děkuju moc za PR!!! 🚀 Mám tam pár drobností v inlinu. Časem bych to udělal jako samostatné moduly, abychom nenutili uživatele tahat webpack i gulp. Jinak se mi to moc líbí.
Ještě bych vás poprosil zrebazovat na main a pak spustit yarn format:fix. Přidal jsem do repozitáře Prettier kvůli formátování, máte jinak nastavený editor a do PR se tak dostalo spousty změn, které ale nejsou chtěné. Měl jsem to udělat už předtím.
Mám z toho moc velkou radost, vidím, že je potřeba ještě udělat dost věcí a vzniklo mi dost TODO. Určitě pište, když tam budou nějaké nejasnosti.
| if (config.builder === "webpack" || options.builder === "webpack") { | ||
| runWebpack(); | ||
| } else if (config.builder === "gulp" || options.builder === "gulp") { | ||
| runGulp(); | ||
| } else { | ||
| console.error("Please specify a valid builder: webpack or gulp"); | ||
| } |
There was a problem hiding this comment.
Prosím, myslím, že by bylo potřeba zachovat status quo, než zavedeme pořádně verzování a publikování jako balíčku. Mohl byste přidat fallback na webpack?
Ještě je přidat options.builder do cli.js.
| const env = config.env; | ||
| const isProduction = (env == "production") ? true : false; |
There was a problem hiding this comment.
Tady se nám trochu rozchází nastavení webpacku a gulpu. Webpack očekává env.production = true|false. Vy tu pracujete s env = "production". Nejspíš to dělá stejnou věc, ale přijde mi to trochu matoucí.
Ještě jsem se chtěl zeptat, jestli počítáte s env, který může uživatel zadat v CLI?
No description provided.