Skip dependency check on ember init
#124
Conversation
| const dependencyChecker = new DependencyChecker(this.project, reporter); | ||
| dependencyChecker.checkDependencies(); | ||
| // When running `ember <command>`, find the `<command>` | ||
| const emberPosition = process.argv.findIndex(arg => arg.endsWith('/ember')); |
There was a problem hiding this comment.
That is a great question, I will investigate!
There was a problem hiding this comment.
I updated AppVeyor’s configuration to run tests both on Linux and Windows, so we have empirical evidence that yes, it seems to work on Windows too! ✨
There was a problem hiding this comment.
Since we have Travis on this project as well, we could do a couple of extra things:
- Either remove Travis and use Appveyor for Windows and Linux,
- Or keep Travis for Linux builds while Appveyor would be used only for Windows builds;
- Or even replace both with a single GitHub Actions that will run tests on both environments.
There was a problem hiding this comment.
Does cli pass invoked command directly to addons? If so, we can use that instead of parsing it.
Other than that, I am good with the changes.
There was a problem hiding this comment.
@quaertym, I’ve explored what I could from the arguments we get, but I couldn’t find anything relevant — which is definitely a shame. 😞
There was a problem hiding this comment.
OK, maybe it can be done later on the cli side. I will approve.
There was a problem hiding this comment.
Let's add GitHub Actions in a separate PR.
gnclmorais
force-pushed
the
master
branch
7 times, most recently
from
b1f0b20 to
f3a6de8
Compare
As described by a [GitHub issue][0], `ember init` would probably make more sense skipping dependency check, so this commit looks into the arguments received and, if the command issued was `init`, skips dependency check and simply displays a warning. [0]: ember-cli/ember-cli#6277
Node 6 is no longer supported as is on this project: ``` $> node -v v6.17.1 $> yarn install --frozen-lockfile --non-interactive yarn install v1.22.17 [1/5] 🔍 Validating package.json... [2/5] 🔍 Resolving packages... [3/5] 🚚 Fetching packages... error [email protected]: The engine "node" is incompatible with this module. Expected version ">=8". Got "6.17.1" error Found incompatible module. info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command. ``` This commit removes version 6 from TravisCI
| const ranWithInit = process.argv[emberPosition + 1] === 'init'; | ||
|
|
||
| if (ranWithInit) { | ||
| process.emitWarning('Skipped dependency checker…'); |
There was a problem hiding this comment.
Hi! Was wondering if this is actually needed. Will this log a warning message in the user's terminal? If so, I think it could be confusing for people new to Ember?
There was a problem hiding this comment.
That’s a good question, could be! However, could new people to Ember also find it confusing that dependency check happens on every command except init, without any warning nor explanation?
There was a problem hiding this comment.
I could be wrong, but I think (most) people (new to Ember) don't know dependency checking happens when running an Ember command, unless it fails. Skipping silently here seems better to me than emitting a warning. There's also nothing the user can do to address it I think.
There was a problem hiding this comment.
Yup, there’s definitely some value to what you said. 👍 Maybe someone else on this PR can chime in…?
There was a problem hiding this comment.
I think dependency-checker should be silent if the things are working as expected. So we can remove the log.
This pull request addresses ember-cli/ember-cli#6277, trying to look into the
process.argvpassed to the Node process to evaluate if we’re looking into anember initexecution. Feedback welcome!As a bonus, in order to get a green CI, I had to remove Node 6 from the versions we test again, since this seems to happen otherwise: