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

fix(import): Better handling of "Patch is empty" #2588

Merged
merged 3 commits into from Nov 8, 2020

Conversation

TimShilov
Copy link
Contributor

@TimShilov TimShilov commented May 18, 2020

Description

Removed check for hard-coded "Patch is empty." git message in favor of git diff COMMIT^! check.
Fixes #2351 , fixes #2587

Motivation and Context

Command lerna import --flatten <path/to/repo> fails if LANG environment variable set to ru_RU.UTF-8 (default value on macOS in Russia).
Example output:

lerna ERR! import Rolling back to previous HEAD (commit fff5b779833c57820df2f76c812fd0e89641de07)
lerna ERR! EIMPORT Failed to apply commit ef18c6c.
lerna ERR! EIMPORT Command failed: git am -3 --keep-non-patch
lerna ERR! EIMPORT Патч пустой.
lerna ERR! EIMPORT Когда вы устраните эту проблему, запустите «git am --continue».
lerna ERR! EIMPORT Если вы хотите пропустить этот патч, то запустите «git am --skip».
lerna ERR! EIMPORT Чтобы вернуться на предыдущую ветку и остановить применение изменений, запустите «git am --abort».
lerna ERR! EIMPORT 
lerna ERR! EIMPORT 
lerna ERR! EIMPORT You may try again with --flatten to import flat history.

How Has This Been Tested?

Updated command's code and successfully imported repo with multiple locales including: en_US.UTF-8, ru_RU.UTF-8, en_IN.utf8, es_ES.UTF-8 and de_CH.ISO8859-1.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • 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.

Given that the import command is a list of hacks to begin with, is there any other way we can know the "emptiness" of the commit? I'm reticent to add a list of hard-coded localized strings, but I understand hard-coded English isn't a better solution.

commands/import/index.js Outdated Show resolved Hide resolved
@TimShilov TimShilov requested a review from evocateur June 1, 2020 16:01
@TimShilov TimShilov changed the title Fix: lerna import fails on Empty patch with LANG=ru_RU.UTF-8 fix(import): Fails on Empty patch with LANG other than en_US.UTF-8 Jul 25, 2020
@TimShilov TimShilov changed the title fix(import): Fails on Empty patch with LANG other than en_US.UTF-8 fix(import): Better handling of "Patch is empty" Jul 25, 2020
@TimShilov
Copy link
Contributor Author

TimShilov commented Jul 25, 2020

@evocateur Hi!
Sorry for pinging you but I really need that fix.
I fixed your change request. Is there anything I can/need to do to get this merged?

@evocateur evocateur changed the base branch from master to main November 8, 2020 00:40
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.

Thanks!

@evocateur evocateur merged commit 0497bc7 into lerna:main Nov 8, 2020
HamishBuckmaster pushed a commit to HamishBuckmaster/lerna that referenced this pull request Jan 21, 2021
Using `git diff COMMIT^!` to check if patch is empty instead of English-only hard-coded string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants