search

OpenFn / kit

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

Fixing Issue:#192 Allow auto-installing metadata for CLI #666

Closed SatyamMattoo closed 5 months ago

SatyamMattoo commented 5 months ago

Short Description

The code changes allow auto-installing metadata for CLI using flags in commands.

Related issue

Fixes #192

Checklist before requesting a review

SatyamMattoo commented 5 months ago

@josephjclark Could you please take a look at the changes I've made for this issue? Your feedback and recommendations would be greatly appreciated. Thanks in advance!

SatyamMattoo commented 5 months ago

Hello @josephjclark,

Thank you for your feedback and guidance. I appreciate your suggestions on improving the quality of my contributions.

I've taken note of your advice regarding reaching out for clarification before starting work and ensuring that I can test the solution effectively. Moving forward, I'll make sure to thoroughly understand the requirements and include proper testing procedures in my contributions.

Once again, thank you for your support and I look forward to working on the updated issue.

Best regards

josephjclark commented 5 months ago

Hi @SatyamMattoo,

We use ava for our unit testing. You'll find many tests in this repo. We generally try to ensure that every file in a src package has a corresponding *.test.ts file in the test folder.

There are more detailed thoughts about testing this particular feature in the issue itself.

But outside of automated tests, my point here is that I don't think you've done anything to try this code out and see if it works. I don't think you've run openfn metadata on your machine :thinking:

While I appreciate that the issue didn't give you enough context to be able to do that, you shouldn't ever submit code to a repo that you haven't run yourself. Otherwise, how do you know if it works? What confidence do you give to the reviewer that your solution works?

I will close down this pull request for now, but please do re-open it or create a new one when you're ready :pray: