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.

Sign up for GitHub

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

Jump to bottom

Detect Duplicates for Singular and Plural Messages and provide merge command #120

Merged
merged 1 commit into from
Nov 15, 2023
Merged

Detect Duplicates for Singular and Plural Messages and provide merge command #120

merged 1 commit into from
Nov 15, 2023

Conversation

maennchen
Copy link
Member

Part of elixir-gettext/gettext#366

Hold off merging until a PR of gettext itself is merged that provides a migration path.

@maennchen maennchen added the enhancement New feature or request label Nov 5, 2023
@maennchen maennchen self-assigned this Nov 5, 2023
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 5, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 039d97de88269db33836787606e08f7bfd50a81c-PR-120

  • 65 of 68 (95.59%) changed or added relevant lines in 6 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 99.284%
Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/expo/message.ex 4 5 80.0%
lib/expo/message/plural.ex 9 10 90.0%
lib/mix/tasks/expo.msguniq.ex 19 20 95.0%
Totals Coverage Status
Change from base Build 74d6e7689eca317dce0667659f0b46e95d1796b0: -0.7%
Covered Lines: 416
Relevant Lines: 419
💛 - Coveralls

@maennchen maennchen marked this pull request as draft November 6, 2023 14:23
@maennchen maennchen force-pushed the jm/singular_plural_duplicates branch from fd231cd to 47e234c Compare November 6, 2023 14:31
@maennchen maennchen changed the title Detect Duplicates for Singular and Plural Messages Detect Duplicates for Singular and Plural Messages and provide merge command Nov 6, 2023
@maennchen maennchen force-pushed the jm/singular_plural_duplicates branch 4 times, most recently from 377ff8d to 50c59c7 Compare November 6, 2023 15:08
@maennchen maennchen mentioned this pull request Nov 6, 2023
Merged
@maennchen maennchen force-pushed the jm/singular_plural_duplicates branch from 50c59c7 to 2439ea3 Compare November 6, 2023 15:32
@maennchen maennchen marked this pull request as ready for review November 6, 2023 15:33
whatyouhide
whatyouhide reviewed Nov 7, 2023
View reviewed changes
Copy link
Contributor

@whatyouhide whatyouhide left a comment

Choose a reason for hiding this comment

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

Started to review this but I stopped because I'd like to first understand if we already have logic for merging messages (I thought we did).

Also, one more thing: let's not use single-letter variables please 🙃 msg1/msg2 goes a long way over a/b.

lib/expo/message.ex Show resolved Hide resolved
lib/mix/tasks/expo.msguniq.ex Outdated Show resolved Hide resolved
lib/expo/message/plural.ex Outdated Show resolved Hide resolved
lib/expo/message/plural.ex Outdated Show resolved Hide resolved
lib/expo/message/plural.ex Show resolved Hide resolved
lib/expo/message/singular.ex Show resolved Hide resolved
lib/expo/message.ex Outdated Show resolved Hide resolved
@maennchen maennchen force-pushed the jm/singular_plural_duplicates branch 3 times, most recently from 6ee7314 to 039d97d Compare November 7, 2023 16:21
@whatyouhide
Copy link
Contributor

@maennchen all makes sense, let's just update the Mix task and lmk when this is ready for another pass of reviewing 💯

@maennchen maennchen force-pushed the jm/singular_plural_duplicates branch from 039d97d to 1fdeb5a Compare November 9, 2023 10:05
@whatyouhide whatyouhide enabled auto-merge (squash) November 15, 2023 07:29
@whatyouhide whatyouhide merged commit e0fa66b into main Nov 15, 2023
11 checks passed
@whatyouhide whatyouhide deleted the jm/singular_plural_duplicates branch November 15, 2023 07:30
@whatyouhide
Copy link
Contributor

🎉

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

@whatyouhide whatyouhide whatyouhide approved these changes

Assignees

@maennchen maennchen

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@maennchen @whatyouhide