Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(version): Skip repository validation when git is unused #1908

Merged
merged 13 commits into from
Feb 14, 2019
Merged

feat(version): Skip repository validation when git is unused #1908

merged 13 commits into from
Feb 14, 2019

Conversation

marikaner
Copy link
Contributor

@marikaner marikaner commented Jan 31, 2019

Do not fail version update when git features are not used.

Description

I am just skipping the check on whether we are @ HEAD when git is not used.

Motivation and Context

closes #1869

How Has This Been Tested?

I tried it with our project, but I did not add any automatic tests. It works in both fixed and locked mode and still throws an error in case git commands are not skipped.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • My code follows the code style of this project.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@evocateur evocateur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no tests, lockfile doesn't need to change, and the problems i described in the linked issue

@marikaner
Copy link
Contributor Author

Thank you for your review, @evocateur. I reverted the changes to the lockfile and will add tests as soon as we agree on the usefulness of this PR ;)

I finally understood your concerns. I misunderstood the purpose of the fixed mode (even though I read your documentation multiple times *) and am now aware that my issue is rather a corner case as you will basically only not need to be on a branch when using --force-publish as well as skipping tagging + pushing.
I still think it might be an improvement as it moves lerna's behaviour closer to npm's (npm does not fail without git).

Please take a look at my current implementation and let me know whether this goes into the right direction in your opinion. I just overrode the requiresGit method and skipped all branch related actions accordingly.

* "Use this if you want to automatically tie all package versions together." confirmed my intuitive understanding...

@marikaner
Copy link
Contributor Author

@evocateur, I added some tests, one is still failing, if will try to get this fixed if I have time this evening, otherwise maybe you can help?

commands/version/index.js Outdated Show resolved Hide resolved
commands/version/index.js Outdated Show resolved Hide resolved
commands/version/index.js Outdated Show resolved Hide resolved
commands/version/index.js Outdated Show resolved Hide resolved
commands/version/index.js Outdated Show resolved Hide resolved
utils/collect-updates/collect-updates.js Outdated Show resolved Hide resolved
commands/version/__tests__/version-command.test.js Outdated Show resolved Hide resolved
@marikaner
Copy link
Contributor Author

@evocateur thank you. I removed forcePublish and independent mode from the conditions for requiresGit. Also, I moved the initialization of the command properties to configureProperties and adjusted the test conditions accordingly.

@marikaner
Copy link
Contributor Author

@evocateur What do you think?

Copy link
Member

@evocateur evocateur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, well done!

utils/collect-updates/collect-updates.js Outdated Show resolved Hide resolved
commands/version/index.js Show resolved Hide resolved
@evocateur
Copy link
Member

I'll merge this once you've rebased.

@evocateur evocateur self-assigned this Feb 13, 2019
@evocateur evocateur changed the title Skip currentBranch validation when git is unused fix(version): Skip branch validation when git is unused Feb 13, 2019
# Conflicts:
#	utils/collect-updates/collect-updates.js
@evocateur evocateur changed the title fix(version): Skip branch validation when git is unused feat(version): Skip repository validation when git is unused Feb 14, 2019
@evocateur evocateur merged commit b9e887e into lerna:master Feb 14, 2019
@evocateur
Copy link
Member

Thanks!

@marikaner
Copy link
Contributor Author

Wohoo!

@lock
Copy link

lock bot commented Apr 15, 2019

This thread has been automatically locked because there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lerna version needs code to be on a branch
2 participants