Skip to content

Conversation

@kdraba
Copy link

@kdraba kdraba commented Jun 1, 2022

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #138 138

What is the new behavior?

The find command respects the langs option and only processes files for the specified languages. If the langs option is not present, all files will be processed.

Does this PR introduce a breaking change?

[x] Yes
[ ] No

Usages of the find command, that specify languages using the langs option will behave differently. Instead of processing all existing file, only those files specified in the langs options will be processed.

Migration: If you want to process all files, remove the langs option from the find command invocation.

Other information

@kdraba kdraba marked this pull request as ready for review June 1, 2022 16:53
@shaharkazaz
Copy link
Collaborator

@kdraba Can you please update the branch with master so we can push this forward? 😃

…i_command_should_respect_langs_option

# Conflicts:
#	src/keys-detective/compare-keys-to-files.ts
@kdraba
Copy link
Author

kdraba commented Mar 6, 2023

@kdraba Can you please update the branch with master so we can push this forward? 😃

Done.

for (const [scope, path] of Object.entries(scopePaths)) {
const keys = scopeToKeys[scope];
if (keys) {
if (keys && typeof path === 'string') {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this needed? path is supposed to be a string

export function findMissingKeys(inlineConfig: Config) {
const logger = getLogger();
const config = resolveConfig(inlineConfig);
const config = resolveConfig({ langs:[], ...inlineConfig });
Copy link
Collaborator

Choose a reason for hiding this comment

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

The only thing is that this config property has 2 different purposes when running the find command and when running the extract.

we need to make it clear in the docs if we want to use the same property.
Also, the default config should be different based on what action you are running, otherwise, you need to override it locally like you did here but I'm not sure that's a good practice in the long run.

filePath: string;
}

export function filterPathByLang(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add an explanation to why this was created and what is this used for

`${res.baseFilesPath}/${isGlobal ? '' : scope}/*.${fileFormat}`
),
)
.filter(filterPathByLang(langsToProcess, scopeAndLangFromPathOption)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like you can create this filter once and use it across the function, generally speaking you can create it once with the command config no?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants