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

Prevent argv check on "add-missing-keys-to-other-language" #13113

Merged

Conversation

DarkSuniuM
Copy link
Contributor

@DarkSuniuM DarkSuniuM commented Apr 12, 2022

What does it do?

Fixes a bug that doesn't affect many users but causes unit tests to fail.

There is a require.main === module in the file, Next to the argv check file,

All I did was moving argv check within the if block to prevent the check when the module is being imported (like in the scripts/front/__tests__/add-missing-keys-to-other-language.test.js)

Why is it needed?

The file is not imported anywhere, I'm not sure if anyone imports updateMissingKeysToJSON from this file (which is a script => scripts/front/add-missing-keys-to-other-language.test.js),

Maybe deleting the module.exports and the require.main === module check is a better refactoring solution,

I agree with you, "Why is it needed?"

How to test it?

Run yarn test:unit in the "main" branch and compare the output of the same command within this PR's branch.

Copy link
Member

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

I mean it makes sense, I honestly have never touched this script before 😆

@derrickmehaffy derrickmehaffy added source: tooling Source is GitHub tooling/tests/ect pr: fix This PR is fixing a bug labels May 19, 2022
@alexandrebodin alexandrebodin requested review from vincentbpro and removed request for alexandrebodin May 31, 2022 11:58
@alexandrebodin alexandrebodin added this to the 4.1.13 milestone May 31, 2022
Copy link
Member

@alexandrebodin alexandrebodin left a comment

Choose a reason for hiding this comment

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

LGTM Thank you for this fix :)

@alexandrebodin alexandrebodin changed the title [BugFix] prevent argv check on "add-missing-keys-to-other-language" Prevent argv check on "add-missing-keys-to-other-language" May 31, 2022
@alexandrebodin alexandrebodin merged commit 4abd4fe into strapi:master May 31, 2022
@DarkSuniuM DarkSuniuM deleted the fix/add-missing-key-script-argv-check branch May 31, 2022 20:44
@Convly Convly modified the milestones: 4.1.13, 4.2.0 Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: fix This PR is fixing a bug source: tooling Source is GitHub tooling/tests/ect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants