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

Use locale pragma instead of POSIX::setlocale() #808

Merged
merged 1 commit into from
Jan 25, 2024
Merged

Conversation

theory
Copy link
Collaborator

@theory theory commented Jan 6, 2024

The Pragma is more likely to do the right thing, as confirmed by new tests, which fail when using setlocale and now succeed with use locale. The tests, in xt/locale, include compiled locale dictionaries (*.mo files) with a single message for the tested languages. This is in contrast to the released locale dictionaries, which are generated at release time but not stored in the repository.

Update the Language-Team header in the project localization packages in po directory to Sqitch Hackers <sqitch-hackers@googlegroups.com>.

Update the os.yml and perl.yml workflows, which run all tests including the new locale tests, to install the required locales on Linux and to set the full runs-on: image name in the matrix (in response to shogo82148/actions-setup-perl#1699).

Also remove the installation of an older version of Locale::TextDomain from those workflows, since gflohr/libintl-perl#7 has been fixed and released.

While at it, upgrade to actions/checkout@v4 in all workflows and use runner.os instead of matrix.os in conditionals.

Fixes #806.

@theory theory self-assigned this Jan 6, 2024
@theory theory marked this pull request as draft January 6, 2024 21:05
@theory theory changed the title Use locale pragma instead of POSIX::set_locale() Use locale pragma instead of POSIX::setlocale() Jan 6, 2024
@coveralls
Copy link

coveralls commented Jan 6, 2024

Pull Request Test Coverage Report for Build 7642397296

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 7634961713: 0.0%
Covered Lines: 4521
Relevant Lines: 4521

💛 - Coveralls

@theory theory force-pushed the perl-locale branch 4 times, most recently from 52d9d8d to 501759c Compare January 8, 2024 00:25
@theory theory force-pushed the perl-locale branch 2 times, most recently from 5f93725 to 633860b Compare January 24, 2024 03:07
@theory theory requested a review from autarch January 24, 2024 15:15
@theory theory marked this pull request as ready for review January 24, 2024 15:17
@theory theory added the bug label Jan 24, 2024
autarch
autarch previously approved these changes Jan 24, 2024
Copy link
Contributor

@autarch autarch left a comment

Choose a reason for hiding this comment

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

LGTM % one typo I found.

use File::Spec;
use Capture::Tiny qw(:all);

# Requires xt/locale/LocaleData; see xt/lcoale/README.md for details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Requires xt/locale/LocaleData; see xt/lcoale/README.md for details.
# Requires xt/locale/LocaleData; see xt/locale/README.md for details.

The Pragma is more likely to do the right thing, as confirmed by new
tests, which fail when using `setlocale` and now succeed with `use
locale`. The tests, in `xt/locale`, include compiled locale dictionaries
(`*.mo` files) with a single message for the tested languages. This is
in contrast to the released locale dictionaries, which are generated at
release time but not stored in the repository.

Update the `Language-Team` header in the project localization packages
in `po` directory to `Sqitch Hackers <sqitch-hackers@googlegroups.com>`.

Update the `os.yml` and `perl.yml` workflows, which run all tests
including the new locale tests, to install the required locales on Linux
and to set the full `runs-on:` image name in the matrix (in response to
shogo82148/actions-setup-perl#1699).

Also remove the installation of an older version of Locale::TextDomain
from those workflows, since gflohr/libintl-perl#7 has been fixed and
released.

While at it, upgrade to `actions/checkout@v4` in all workflows and use
`runner.os` instead of `matrix.os` in conditionals.
@theory theory requested a review from autarch January 24, 2024 22:49
@theory theory merged commit 86a33b1 into develop Jan 25, 2024
183 checks passed
@theory theory deleted the perl-locale branch January 25, 2024 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants