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

fix: respect --cwd argument when generating/extracting translations #540

Merged
merged 5 commits into from
Apr 27, 2021
Merged

fix: respect --cwd argument when generating/extracting translations #540

merged 5 commits into from
Apr 27, 2021

Conversation

mediremi
Copy link
Contributor

Currently, when generating i18n strings (either as a result of d2-app-scripts start or d2-app-scripts extract), a message in the format below is logged:

Writing N language strings to .//home/user/some/path/app-being-developed/i18n/en.pot...

This PR ensures that the path to en.pot logged is relative to the current working directory, resulting in the more consice:

Writing N language strings to ./en.pot...

@mediremi mediremi requested a review from amcgee March 24, 2021 12:00
amcgee
amcgee approved these changes Mar 24, 2021
View reviewed changes
Copy link
Member

@amcgee amcgee left a comment

Choose a reason for hiding this comment

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

LGTM

i see there's actually an existing bug here - the CLI commands accept a --cwd argument, so process.cwd() is NOT always guaranteed to be the proper cwd. This function should probably accept paths which includes the cwd arg (defaulting to process.cwd()) and use that instead - as is currently written, this change looks correct. Feel free to fix the larger issue if desired, but if we want to defer the bugfix (likely also in generate as both were ported from https://github.com/dhis2/d2-i18n-generate / https://github.com/dhis2/d2-i18n-extract) we just can create an issue to follow up.

@mediremi
Copy link
Contributor Author

mediremi commented Mar 24, 2021 •
edited
Loading

I'll update both extract and generate so that they use paths.base instead of process.cwd() 👍

@mediremi mediremi changed the title fix: show relative path when logging i18n strings output destination fix: respect --cwd argument when generating/extracting translations Mar 24, 2021
@mediremi mediremi requested a review from amcgee March 24, 2021 14:37
amcgee
amcgee requested changes Mar 29, 2021
View reviewed changes
cli/src/commands/i18n.js Outdated Show resolved Hide resolved
@mediremi mediremi requested a review from amcgee March 30, 2021 10:02
@mediremi mediremi merged commit 280e02c into master Apr 27, 2021
@mediremi mediremi deleted the fix/i18n-path-log branch April 27, 2021 07:40
dhis2-bot added a commit that referenced this pull request Apr 27, 2021
@dhis2-bot
chore(release): cut 6.1.1 [skip ci]
5647e24 
## [6.1.1](v6.1.0...v6.1.1) (2021-04-27)

### Bug Fixes

* respect --cwd argument when generating/extracting translations ([#540](#540)) ([280e02c](280e02c))
@dhis2-bot
Copy link
Contributor

🎉 This PR is included in version 6.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

@amcgee amcgee amcgee approved these changes

Assignees
No one assigned
Labels
released
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mediremi @dhis2-bot @amcgee