Fix yarn version which yields same output as yarn --version (#2491)
#2510
Conversation
|
tests got broken |
|
Yeah; odd because running Is that the best way to ensure testing is smooth? Or am I missing something else? Pardon my ignorance since this is the first time I've delved into the Yarn source. 😄 |
|
@ConAntonakos, the CI could fail because of network issues or was based on an unstable commit. |
|
Sure! Thanks for letting me know. |
ConAntonakos
force-pushed
the
2491-restore-yarn-version-cmd
branch
from
7f20770
to
5a55b7d
Compare
|
I think CI shows a genuine break in tests now |
|
Since |
|
@markstos That's a good idea. I essentially reverted a previous change that caused the break. I'll keep at it and consider test coverage as well. |
|
Ok, I was able to get unit tests working again. It seems they weren't playing nicely with node v7, which breaks them. I'll get a screen capture of the error output next time. So I downgraded to node v6.9.x. I'm delving more into the source - and subsequently, the The major culprit is this line: https://github.com/yarnpkg/yarn/blob/master/src/cli/index.js#L174 If the above line is removed, |
ConAntonakos
force-pushed
the
2491-restore-yarn-version-cmd
branch
from
437847f
to
b2c4717
Compare
|
To rebase properly I usually do Merging a master branch does not work well with github PRs |
ConAntonakos
force-pushed
the
2491-restore-yarn-version-cmd
branch
from
b2c4717
to
7772b00
Compare
|
Ugh! I apologize for that. Do you want me to redo that? |
|
No worries, yeah, better rebase, otherwise I don't know what happens when the PR is merged :) |
…pkg#2491) There was a conflict when commander attempts to parse the incoming args between the command executed and the options since the name `version` was shared. In other words, executing the command `yarn version` would yield the same output as `yarn --version`. This relates to PR yarnpkg#2268.
…ting name bug Relating to tj/commander.js#346, when an arg shares the same name as an option, it wrongly ignores the arg command and executes the option instead. Therefore, executing 'yarn version' would instead translate to 'yarn --version'. This logic can subsequently be removed once this issue is resolved.
ConAntonakos
force-pushed
the
2491-restore-yarn-version-cmd
branch
from
7772b00
to
5984b51
Compare
|
Looks like it's failing the following test type
Also, TODO: Add tests (or, improve tests) for |
|
@ConAntonakos, don't worry about the MacOS build on travis, it is a bit flaky. |
|
I'll merge the fix now. |
… (#2510) * Fix `yarn version` which yields same output as `yarn --version` (#2491) There was a conflict when commander attempts to parse the incoming args between the command executed and the options since the name `version` was shared. In other words, executing the command `yarn version` would yield the same output as `yarn --version`. This relates to PR #2268. * Shift first arg that shares name with an option to circumvent conflicting name bug Relating to tj/commander.js#346, when an arg shares the same name as an option, it wrongly ignores the arg command and executes the option instead. Therefore, executing 'yarn version' would instead translate to 'yarn --version'. This logic can subsequently be removed once this issue is resolved.
… (#2510) * Fix `yarn version` which yields same output as `yarn --version` (#2491) There was a conflict when commander attempts to parse the incoming args between the command executed and the options since the name `version` was shared. In other words, executing the command `yarn version` would yield the same output as `yarn --version`. This relates to PR #2268. * Shift first arg that shares name with an option to circumvent conflicting name bug Relating to tj/commander.js#346, when an arg shares the same name as an option, it wrongly ignores the arg command and executes the option instead. Therefore, executing 'yarn version' would instead translate to 'yarn --version'. This logic can subsequently be removed once this issue is resolved.
|
@bestander Indeed! I want to digest the structure of the testing (and subsequently, the flow of logic) and figure out why they weren't catching this error. |
|
This is very odd. Instantiating |
…pkg#2491) (yarnpkg#2510) * Fix `yarn version` which yields same output as `yarn --version` (yarnpkg#2491) There was a conflict when commander attempts to parse the incoming args between the command executed and the options since the name `version` was shared. In other words, executing the command `yarn version` would yield the same output as `yarn --version`. This relates to PR yarnpkg#2268. * Shift first arg that shares name with an option to circumvent conflicting name bug Relating to tj/commander.js#346, when an arg shares the same name as an option, it wrongly ignores the arg command and executes the option instead. Therefore, executing 'yarn version' would instead translate to 'yarn --version'. This logic can subsequently be removed once this issue is resolved.
There was a conflict when commander attempts to parse the incoming args between the command executed and the options since the name
versionwas shared. In other words, executing the commandyarn versionwould yield the same output asyarn --version. This fixes #2491. This also seems to relate to PR #2268.Let me know what you think. And sorry for the issue reference spam! Thanks!