feat: add possibility to override default path to config file#667
feat: add possibility to override default path to config file#667whalemare wants to merge 2 commits intocodingcommons:mainfrom
Conversation
ivanhofer
left a comment
ivanhofer
left a comment
There was a problem hiding this comment.
Thanks for the PR. I haven't tested it yet. I just took a look at the changed files and added a few comments. Please make sure that the exporter and importer functionality works too :)
| const config = await getConfigDiff(options) | ||
|
|
||
| await writeConfigToFile(config) | ||
| await writeConfigToFile(config, '.typesafe-i18n.json') |
There was a problem hiding this comment.
don't you need to use options.project here too?
There was a problem hiding this comment.
When I wrote this, I think that this setup method is used only for in-place config generation, not for generate config in path passed by --project path
We can add path here too if needed
There was a problem hiding this comment.
I think it would be more consistent if someone is able to pass the -p flag also to the setup function.
| logger.info(`version ${version}`) | ||
|
|
||
| await checkAndUpdateSchemaVersion() | ||
| await checkAndUpdateSchemaVersion(options.project) |
There was a problem hiding this comment.
can we add a validation here?
The string should end with .json and we should check if the file exists (except if it is a setup call).
|
|
||
| export const readConfig = async (): Promise<GeneratorConfig> => { | ||
| const generatorConfig = await readRawConfig() | ||
| export const readConfig = async (configPath = '.typesafe-i18n.json'): Promise<GeneratorConfig> => { |
There was a problem hiding this comment.
I would not use default values anywhere. This makes it easy to miss a spot where we forgot to pass it.
There was a problem hiding this comment.
Did you have some ideas, how we can save backward compatibility with previous codebase that have hardcoded path to file, without default values?
There was a problem hiding this comment.
Ideally it get's passed from the CLI and .typesafe-i18n.json is the default value there if -p does not get specified.
Without having a look at the code, I currently don't really know where this function get's called. But the call stack to this function should only be initiated from the CLI or the exporter & importer I think
|
@whalemare I saw this PR was not updated for a while. Do you plan to continue working on this PR? |
Yes, but little bit early. We can close this PR if you want and I reopen it or create new one, when I will be ready |
|
No it's fine. We can keep it open if you plan to continue working on it. |
2 participants
Related to #656
Allow pass additional parameter to
clinamed as--project, that allow users to override default'.typesafe-i18n.json'path to config fileSave backward compatibility to allow current users change nothing in it's workflow.
Tested locally with command