longportapp / steampipe-plugin-longport

Steampipe plugin for Query from LongPort OpenAPI.
Apache License 2.0
4 stars 0 forks source link

Initial suggestions for plugin release #2

Open madhushreeray30 opened 6 months ago

madhushreeray30 commented 6 months ago

Thanks @huacnlee for this new plugin. Great work 🎉 !!

The basic structure looks good so far. While using the plugin, we did come up with a few suggestions based on our best practices:

export LONGPORT_APP_KEY=a12b34cd-56ef-78gh-9i00-jk123lmn456o export LONGPORT_APP_SECRET=sEcReT-9876-AbCd-5432-EfGhIjKlMnOp export LONGPORT_ACCESS_TOKEN=token_1234abcd5678efgh9012ijkl


`README.md`
- Could you please update the file to follow the format of the [namecheap plugin](https://github.com/turbot/steampipe-plugin-namecheap/blob/main/README.md).
- The basic sections are missing so it would be better if you could follow the format from the above reference and then we can further see if any changes are required.

- [x] `longport/columns.go`

- We generally keep the columns in the respective table codes and if there are common columns used across we add it to a `common_collumns.go` file. Could you please shift the columns to the respective tables and if any common column is used keep it here after changing the file name to `common_collumns.go`.
- Each column description should end with a `.`
- Do we need to add the transform function in all the columns? The default conversation is from camel case to snake case so I think we do not need the transform when we are keeping the column names same just changing from camel case to snake case. Eg > `TradeStatus` would be changed by GO to `trade_status` without any transform function.

- [x] `longport/connection_config.go`

- Please update the file name from `config.go` to `connection_config.go`
- Update the `cty` to `hcl` and remove the`ConfigSchema` (as per our latest changes across all plugins)

- [x] `longport/longport_*.go`

- Please give the table description in the form of a complete sentence. For some tables, it is a bit messy.
- Please update the file name to use singular naming for example `longport_broker.go`
- Please update the function name to the format `tableLongportBroker`. In some tables, we have followed the `longport_history_executions` format which should also be updated to `tableLongportHistoryExecutions`. Apply to all tables.
- Whenever we get an error we should print the type of error in the logger for example `plugin.Logger(ctx).Error("longport_broker.listBrokers", "connection_error", err)` when the error is during client creation and `plugin.Logger(ctx).Error("longport_broker.listBrokers", "api_error", err)` when the error is during the API call. Please apply this to all tables. ([reference](https://github.com/turbot/steampipe-plugin-namecheap/blob/c66bf5df9f8029ff390ca902c79e42812d451da9/namecheap/table_namecheap_domain.go#L46))

- [x] `longport/plugin.go`

- The table names should be sorted in ascending order. (Make sure to use the updated function names).
- We no longer need the `ConfigSchema` so we can remove it.

- [x] `.github`

- The folder does not have all the required files can you please update it taking reference from [here](https://github.com/turbot/steampipe-plugin-namecheap/tree/main/.github).

- [x] `DEVELOPMENT.md` and `version.json`

- We do not require these files as the information in README and index docs are sufficient.

- [x] `go.mod`

- Please update the steampipe-plugin-sdk to `v5.8.0`

- [x] `CHANGELOG.md`

- Please draft an initial CHANGELOG that will include the following 

v0.0.1 [TBD]

What's new?

Please let us know if you have questions, happy to help 👍.

huacnlee commented 6 months ago

@madhushreeray30 😂 Looks like too many rules, I will try to update them.

But should I need to update all of them?

madhushreeray30 commented 6 months ago

@huacnlee Apologies for the late reply, It seems like a lot of rules however this is the basic structure we follow for all the plugins. It would be great if you could update the plugin as required. Thanks!

huacnlee commented 6 months ago

I can't write all table's complete documentation in right now, because there have a lot of details of each table, it related to the business logic on https://open.longportapp.com. And as steampipe-plugin the users just need to know the table's name and the columns, so I think it's enough for now.

The other things I think I have done before last commit, if the doc can ignore please check out it again.