search

OData / ODataConnectedService

A Visual Studio extension for generating client code for OData Services
Other
79 stars 41 forks source link

Add command-line option for ConnectedService.json file #363

Closed gregwinterstein closed 5 months ago

gregwinterstein commented 1 year ago

Add command-line option for reading generation configuration from ConnectedService.json file Command-line option: --connected-service-file or -c

Fixes #338 Fixes #327

gregwinterstein commented 1 year ago

Rebased to resolve conflicts in GenerateCommand.cs

KenitoInc commented 10 months ago

@gregwinterstein Kindly look into review comments by @gathogojr

gregwinterstein commented 10 months ago

@gregwinterstein Kindly look into review comments by @gathogojr

Hello @KenitoInc , I wlll respond as soon as I can. You all have caught me at an extremely busy time

gregwinterstein commented 9 months ago

@gregwinterstein Kindly look into review comments by @gathogojr

Hello @KentoInc & @gathogojr, I have reviewed your comments and updated the pull request as requested.

jvalkeejarvi commented 8 months ago

Hi! What is the situation of this PR? I'd really like to get the ConnectedService.json file support for CLI.

wachugamaina commented 6 months ago

Could we add some tests to this Pr ?

gregwinterstein commented 6 months ago

@wachugamaina

Could we add some tests to this Pr ?

This pull request adds a command-line option for reading generation configuration from ConnectedService.json file Command-line option: --config-file or -c

The overwhelming majority of the changes are in src/Microsoft.OData.Cli/GenerateCommand.cs, inside private methods.

What would you like to see tested in this pull request? How would you propose to test the changes in this pull request?

habbes commented 5 months ago

We do need to improve the test setup for the CLI, not just this PR (cc @ElizabethOkerio). See: https://github.com/OData/ODataConnectedService/issues/254

What would be a good starting point in testing the CLI is that the CLI options are correctly transformed into the expected ServiceConfig. And for the purpose of this PR, we would test whether the config in the JSON config files are correctly applied to the ServiceConfig. That would require some refactoring and setting a test project. The next step would be to consider some E2E tests, running the CLI and checking the results. Maybe that's beyond the scope of this PR? If it is, then something we should prioritize with a sense of urgency.

gregwinterstein commented 5 months ago

We do need to improve the test setup for the CLI, not just this PR (cc @ElizabethOkerio). See: #254

What would be a good starting point in testing the CLI is that the CLI options are correctly transformed into the expected ServiceConfig. And for the purpose of this PR, we would test whether the config in the JSON config files are correctly applied to the ServiceConfig. That would require some refactoring and setting a test project. The next step would be to consider some E2E tests, running the CLI and checking the results. Maybe that's beyond the scope of this PR? If it is, then something we should prioritize with a sense of urgency.

@habbes I think that given that most of the CLI options are not currently tested, this should be out of scope for this pull request. At this point, I do not have the time to implement tests for the CLI options.

gathogojr commented 5 months ago

@gregwinterstein Please take note of the following two review comments. Not sure you saw them since they were collapsed:

gregwinterstein commented 5 months ago

@gregwinterstein Please take note of the following two review comments. Not sure you saw them since they were collapsed:

* [Add command-line option for ConnectedService.json file #363 (comment)](https://github.com/OData/ODataConnectedService/pull/363#discussion_r1537580774)

* [Add command-line option for ConnectedService.json file #363 (comment)](https://github.com/OData/ODataConnectedService/pull/363#discussion_r1537610589)

@gathogojr Thank you for the reminder. I have replied to both of the comments you mentioned above. Please review.