OpenFn / kit

The bits & pieces that make OpenFn work. (diagrammer, cli, compiler, runtime, runtime manager, logger, etc.)
8 stars 12 forks source link

Fixing: #604 #695

Closed SatyamMattoo closed 1 month ago

SatyamMattoo commented 1 month ago

Short Description

Disables logging of env variables and populates the args according to their priority.

Related issue

Fixes #604

Implementation Details

Added a to function to populate the args in a different cli.ts file. Default values are not provided to the yargs but are added when the function returns the values. This also ensures the env variables are not logged on pnpm start --help .

QA Notes

I have added tests related to this function in cli.test.ts file. More tests can be added if needed.

Checklist before requesting a review

josephjclark commented 1 month ago

Hey @SatyamMattoo, here's what we'll do here:

I do agree that it's a bit nicer to isolate the yargs stuff in one place. This approach keeps the cli stuff clean and discoverable, but de-prioritises its importance a little. This helps avoid the confusion with how other packages deal with cli.ts.

SatyamMattoo commented 1 month ago

Hey @josephjclark I have made the changes accordingly, you can review them at your convenience and let me know if there are any more changes needed.

Best regards.

josephjclark commented 1 month ago

@SatyamMattoo Thank you for this, it looks great. I'll merge it in and prepare it for release this week.

For future reference, please try to avoid commit messages like requested changes. You should pretend like I didn't ask for the changes and instead document (broadly) what you did.