Skip to content

Order extracted messages alphabetically by message ID#230

Merged
tricoder42 merged 2 commits into
lingui:stable-2.xfrom
queicherius:stable-2.x
Jun 24, 2018
Merged

Order extracted messages alphabetically by message ID#230
tricoder42 merged 2 commits into
lingui:stable-2.xfrom
queicherius:stable-2.x

Conversation

@queicherius
Copy link
Copy Markdown
Contributor

@queicherius queicherius commented Jun 20, 2018

This is my try to implement #229.

  • I did not work with ramda before, so I am sure there is a better way to implement orderByMessageId.
  • Only a part of the tests ran locally for me, so I'm going to see what CI says
  • I also did not test the actual output of the built command, since the build failed for me locally
  • Feel free to completely discard this if there is a better way to implement it 😄

@tricoder42
Copy link
Copy Markdown
Contributor

I did not work with ramda before, so I am sure there is a better way to implement orderByMessageId.

Me neither, looks good!

I also did not test the actual output of the built command, since the build failed for me locally.

How the build failed? Would you like to share the output?

Feel free to completely discard this if there is a better way to implement it

No, it's awesome! Thanks, going to merge and release it soon.

@tricoder42 tricoder42 merged commit 93fcd9b into lingui:stable-2.x Jun 24, 2018
@queicherius
Copy link
Copy Markdown
Contributor Author

Thanks for merging! :) Looking forward to this.

How the build failed? Would you like to share the output?

I use Linux for windows, and sometimes builds just fail for me on that setup. I attached the output of node ./scripts/test.js here: test-output.txt

@tricoder42
Copy link
Copy Markdown
Contributor

Fixed and released in v2.2.0. Thanks to all involved!

@tricoder42
Copy link
Copy Markdown
Contributor

@queicherius hmm, weird. This looks like something in lodash or probably lodash/windows combination :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants