Skip to content

Conversation

@flodolo
Copy link
Contributor

@flodolo flodolo commented Mar 15, 2018

No description provided.

@flodolo flodolo requested a review from stasm March 15, 2018 09:35
b(message), user=b(author), addremove=True
)
except hglib.error.CommandError as err:
print(' WARNING: hg commit failed ({})'.format(err))
Copy link
Contributor

Choose a reason for hiding this comment

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

In case of Mercurial errors, I think it would make sense to actually interrupt the migration. Should we check if there's anything to commit before we even print Committing changeset in line 85?

Copy link
Contributor Author

@flodolo flodolo Mar 15, 2018

Choose a reason for hiding this comment

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

So far I've only found a case where this happens, and that's me playing with the local files.

Unless there are other use cases, I'm not sure we should interrupt it? The print should likely be move after the commit

The output currently looks something like this:

Running migration bug_1435912_preferences_general_xul for it
  Writing to /Users/flodolo/mozilla/mercurial/l10n_clones/locales/it/browser/browser/preferences/preferences.ftl
    Committing changeset: Bug 1435912 - Migrate Preferences::General XUL to Fluent, part 1.
  WARNING: hg commit failed ((1, 'invalid branchheads cache (served): tip differs\nnothing changed', ''))

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, fair enough :) Can you change the indentation of the warning as follows?

    Committing changeset: Bug 1435912 - Migrate Preferences::General XUL to Fluent, part 1.
      WARNING: hg commit failed…

@flodolo flodolo merged commit 9030138 into projectfluent:master Mar 15, 2018
@flodolo flodolo deleted the bug1444999 branch March 15, 2018 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants