-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(compogen): use jsonref when generating the README #99
Conversation
43a96e8
to
90f1cd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking care of this 💛
A couple of changes are required:
- The tests aren't passing with these changes, they need to be updated
- It'd be good to add a GHA for this package using
make test
- It'd be good to add a GHA for this package using
- We need to add the generation tags to the components that can have autogenerated documentation. That way, the integration action Naman is working on will take them into account.
If you prefer it, I can have a look at this items and contribute to this branch.
@@ -14,6 +14,6 @@ type property struct { | |||
} | |||
|
|||
type objectSchema struct { | |||
Properties map[string]property `json:"properties" validate:"gt=0,dive"` | |||
Properties map[string]property `json:"properties" validate:"dive"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather avoid empty objects in the definitions and keep this validation but if that's not possible I'm fine with loosening the restrictions. Did you encounter any other case with empty object schemas apart from resource_specification
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me do a quick try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Instill Model connector will encounter some issues with this (I guess it is caused by oneof
). However, we are also discussing removing the "external mode" for the Instill Model connector. The problem will be resolved if we decide to remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into it 🙇 In order not to be blocked by the removal of external mode, I'd remove the gt=0
validation with a TODO comment
Because - `compogen` tests are broken This commit - fixes tests - adds action to check `compogen` when changed
🤖 I have created a release *beep* *boop* --- ## [0.15.0-beta](v0.14.1-beta...v0.15.0-beta) (2024-04-25) ### Features * add `UsageHandler` interface ([#87](#87)) ([b9d9645](b9d9645)) * adjust `IConnector` interface ([#83](#83)) ([46ea796](46ea796)) * adjust `Test()` interface ([#81](#81)) ([763cc6d](763cc6d)) * adopt system variables ([#92](#92)) ([e8ae4e1](e8ae4e1)) * expose `IsCredentialField` interface ([#93](#93)) ([6cd2801](6cd2801)) * merge resource spec into component spec ([#86](#86)) ([a6de70e](a6de70e)) * **airbyte:** remove Airbyte connectors ([#88](#88)) ([ec770d6](ec770d62cf52fa099a9be6825432d76f0d211f4a))* * **compogen:** use jsonref when generating the README ([#99](#99)) ([ff49157](ff49157)) * **instill:** drop support for "external mode" ([#101](#101)) ([b0c091b](b0c091b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Because - The `compogen` tool didn't use `jsonref` to render the definition, some component readme cannot be generated. This commit - Uses `jsonref` when generating the README. - Renames `resource_specification` to `connection_specification`. - Removes the validation for an empty `connection_specification`. Note - We still do not support nested structures yet. --------- Co-authored-by: Juan Vallés <3977183+jvallesm@users.noreply.github.com> Co-authored-by: Juan Vallés <jvallesm@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [0.15.0-beta](instill-ai/component@v0.14.1-beta...v0.15.0-beta) (2024-04-25) ### Features * add `UsageHandler` interface ([instill-ai#87](instill-ai#87)) ([b9d9645](instill-ai@b9d9645)) * adjust `IConnector` interface ([instill-ai#83](instill-ai#83)) ([46ea796](instill-ai@46ea796)) * adjust `Test()` interface ([instill-ai#81](instill-ai#81)) ([763cc6d](instill-ai@763cc6d)) * adopt system variables ([instill-ai#92](instill-ai#92)) ([e8ae4e1](instill-ai@e8ae4e1)) * expose `IsCredentialField` interface ([instill-ai#93](instill-ai#93)) ([6cd2801](instill-ai@6cd2801)) * merge resource spec into component spec ([instill-ai#86](instill-ai#86)) ([a6de70e](instill-ai@a6de70e)) * **airbyte:** remove Airbyte connectors ([instill-ai#88](instill-ai#88)) ([ec770d6](instill-ai@ec770d62cf52fa099a9be6825432d76f0d211f4a))* * **compogen:** use jsonref when generating the README ([instill-ai#99](instill-ai#99)) ([ff49157](instill-ai@ff49157)) * **instill:** drop support for "external mode" ([instill-ai#101](instill-ai#101)) ([b0c091b](instill-ai@b0c091b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Because
compogen
tool didn't usejsonref
to render the definition, some component readme cannot be generated.This commit
jsonref
when generating the README.resource_specification
toconnection_specification
.connection_specification
.Note