chore(cli): remove findup-sync from root move to utils package && format ts files #806

anshumanv · 2019-03-27T13:38:32Z

remove findup-sync from package dir and move to utils

ISSUES CLOSED: #805

What kind of change does this PR introduce?
refactor mostly

Did you add tests for your changes?
yes

If relevant, did you update the documentation?
NA

Summary
removed findup-sync package from root, ported to utils

Does this PR introduce a breaking change?
Yes

Other information

* chore(cli): move constants to a separate file moved all constants to a separate file for CLI scopre and imported from there. ISSUES CLOSED: webpack#772 * chore(cli): lint files, rm console.log Format all code using prettier, remove console.log ISSUES CLOSED: webpack#772 * chore(cli): codacy fix codacy fix * chore(cli): split destructuring into multiple lines split constants destructuring to multiple lines in config-yargs * chore(cli): update var name, revert oc update var name, revert oc * chore(cli): moved constants to utils moved constants to utils directory

bin/utils/convert-argv.js

ematipico

Please import the utility as it was a package

packages/utils/webpack-config-path.ts

anshumanv · 2019-03-30T14:04:58Z

Can we possibly disable Travis comments as everyone can just follow the stack trace on Travis page? Comments seem to create a lot of noise. 😅

ematipico · 2019-03-31T07:30:29Z

Sometimes people don't know how get to Travis logs and also from mobile it's not easy to read it. Here it helps and you also get a notification.

anshumanv · 2019-03-31T07:32:04Z

@ematipico makes sense, can you help me figure out why tests are failing? Probably I'm missing something simple. 🤔

bin/utils/convert-argv.js

anshumanv · 2019-04-05T17:52:52Z

@ematipico still no help, it installs the npm version and not the local version, can you suggest something?

misterdev · 2019-04-05T18:27:46Z

Have you tried npm link inside utils and then npm link @webpack-cli/utils where you want to use it?
It should also be added as a dependency :)

Update: I'm not from my PC so I may have said obvious things, if it is failing in the CI I don't think it can work without republishing the utils package

anshumanv · 2019-04-05T18:39:58Z

Yeah works that way, locally. How do we make it work here? 🤔

ematipico · 2019-04-05T22:02:42Z

You have to run Lerna bootstrap in order to link correctly the packages

anshumanv · 2019-04-06T07:07:30Z

So the key is about listing @webpack-cli/utils as a local depency which installs from file, but that would mean every inter package dependency needs to be converted to local dep which will be installed from the local package and not from npm, should we go for it @ematipico

ematipico · 2019-04-07T08:59:40Z

The real issue is that during the CI we don't call the command npm run bootstrap to link the packages before running the tests. We don't need to do any sort of link or local dep as you mentioned.

On your machine works because you linked correctly all the packages by running the command npm run bootstrap but it looks like we don't do the same on Travis/Azure. So I think, order to fix your issue, we just need to run that command before running the tests.

anshumanv · 2019-04-07T12:39:38Z

If I understand correctly it seems we are running it @ematipico -

webpack-cli/azure-pipelines-template.yml

Line 24 in b96857f

npm run bootstrap

🤔

ematipico · 2019-04-07T14:18:42Z

That's weird 😮

bin/utils/convert-argv.js

cleanup

packages/utils/__tests__/is-local-path.test.ts

use absolute path

webpack-bot · 2019-04-23T05:10:39Z

@anshumanv Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@evenstensberg Please review the new changes.

use relative path for now

anshumanv · 2019-04-26T14:37:02Z

@evenstensberg PTAL ✨

#806 (comment)

anshumanv · 2019-05-15T03:45:13Z

@evenstensberg it's recommended to use relative imports for tests - lerna/lerna#2052 (comment)

evenstensberg

Nearly there

packages/utils/path-utils.ts

webpack-bot · 2019-05-30T17:28:33Z

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

evenstensberg

lgtm, thanks for taking the time! 💙

anshumanv added 2 commits March 25, 2019 11:56

Merge branch 'master' of https://github.com/webpack/webpack-cli

a26a48c

webpack-bot added the PR: unreviewed label Mar 27, 2019

anshumanv commented Mar 27, 2019

View reviewed changes

bin/utils/convert-argv.js Outdated Show resolved Hide resolved

webpack-bot added the PR: CI-ok label Mar 27, 2019

anshumanv changed the title ~~chore(cli): remove findup-sync from package dir and move to utils~~ chore(cli): remove findup-sync from root move to utils package Mar 27, 2019

ematipico suggested changes Mar 30, 2019

View reviewed changes

webpack-bot added PR: reviewed-changes-requested and removed PR: unreviewed labels Mar 30, 2019

ematipico reviewed Mar 30, 2019

View reviewed changes

packages/utils/webpack-config-path.ts Outdated Show resolved Hide resolved

webpack-bot added PR: review-outdated PR: CI-not-ok and removed PR: reviewed-changes-requested PR: CI-ok labels Mar 30, 2019

evenstensberg reviewed Apr 3, 2019

View reviewed changes

bin/utils/convert-argv.js Outdated Show resolved Hide resolved

anshumanv force-pushed the config-root branch from c537f4b to 7f71c79 Compare April 5, 2019 17:42

ematipico closed this Apr 7, 2019

ematipico reopened this Apr 7, 2019

Merge branch 'master' of https://github.com/webpack/webpack-cli

6d75e25

dhruvdutt approved these changes Apr 20, 2019

View reviewed changes

bin/utils/convert-argv.js Outdated Show resolved Hide resolved

chore(cli): cleanup

ed4095f

cleanup

webpack-bot added PR: review-outdated and removed PR: reviewed-approved labels Apr 20, 2019

evenstensberg requested changes Apr 21, 2019

View reviewed changes

packages/utils/__tests__/is-local-path.test.ts Show resolved Hide resolved

webpack-bot added PR: reviewed-changes-requested and removed PR: review-outdated labels Apr 21, 2019

chore(utils): use absolute path

00a6348

use absolute path

webpack-bot added PR: review-outdated and removed PR: reviewed-changes-requested labels Apr 23, 2019

chore(util): use relative for now

5a0952e

use relative path for now

evenstensberg requested changes May 28, 2019

View reviewed changes

packages/utils/path-utils.ts Outdated Show resolved Hide resolved

packages/utils/path-utils.ts Outdated Show resolved Hide resolved

webpack-bot added PR: reviewed-changes-requested and removed PR: review-outdated labels May 28, 2019

evenstensberg added 3 commits May 30, 2019 18:57

chore: format and revise code w.r.t style

e9d426d

chore: merge against master

43c00e0

fix: formatting files

eb3909b

evenstensberg changed the title ~~chore(cli): remove findup-sync from root move to utils package~~ chore(cli): remove findup-sync from root move to utils package && format ts files May 30, 2019

chore: fix linting

c7c1a83

webpack-bot added PR: CI-not-ok PR: CI-ok and removed PR: CI-ok PR: CI-not-ok labels May 30, 2019

evenstensberg approved these changes May 30, 2019

View reviewed changes

webpack-bot added the PR: reviewed-approved label May 30, 2019

evenstensberg merged commit f6e57a8 into webpack:master May 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(cli): remove findup-sync from root move to utils package && format ts files #806

chore(cli): remove findup-sync from root move to utils package && format ts files #806

anshumanv commented Mar 27, 2019

ematipico left a comment

anshumanv commented Mar 30, 2019

ematipico commented Mar 31, 2019

anshumanv commented Mar 31, 2019

anshumanv commented Apr 5, 2019

misterdev commented Apr 5, 2019 •

edited

Loading

anshumanv commented Apr 5, 2019 •

edited

Loading

ematipico commented Apr 5, 2019

anshumanv commented Apr 6, 2019

ematipico commented Apr 7, 2019 •

edited

Loading

anshumanv commented Apr 7, 2019 •

edited

Loading

ematipico commented Apr 7, 2019

webpack-bot commented Apr 23, 2019

anshumanv commented Apr 26, 2019

anshumanv commented May 15, 2019

evenstensberg left a comment

webpack-bot commented May 30, 2019

evenstensberg left a comment

chore(cli): remove findup-sync from root move to utils package && format ts files #806

chore(cli): remove findup-sync from root move to utils package && format ts files #806

Conversation

anshumanv commented Mar 27, 2019

ematipico left a comment

Choose a reason for hiding this comment

anshumanv commented Mar 30, 2019

ematipico commented Mar 31, 2019

anshumanv commented Mar 31, 2019

anshumanv commented Apr 5, 2019

misterdev commented Apr 5, 2019 • edited Loading

anshumanv commented Apr 5, 2019 • edited Loading

ematipico commented Apr 5, 2019

anshumanv commented Apr 6, 2019

ematipico commented Apr 7, 2019 • edited Loading

anshumanv commented Apr 7, 2019 • edited Loading

ematipico commented Apr 7, 2019

webpack-bot commented Apr 23, 2019

anshumanv commented Apr 26, 2019

anshumanv commented May 15, 2019

evenstensberg left a comment

Choose a reason for hiding this comment

webpack-bot commented May 30, 2019

evenstensberg left a comment

Choose a reason for hiding this comment

misterdev commented Apr 5, 2019 •

edited

Loading

anshumanv commented Apr 5, 2019 •

edited

Loading

ematipico commented Apr 7, 2019 •

edited

Loading

anshumanv commented Apr 7, 2019 •

edited

Loading