marekjalovec / steampipe-plugin-make Goto Github PK

Hey @marekjalovec, great work on the new plugin 👍.

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

1. The column descriptions of tables appear to be incorrect. For instance, in https://github.com/marekjalovec/steampipe-plugin-make/blob/main/make/table_make_api_token.go#L21-L24 the column names do not match the description. Could you please update them? Certain descriptions are incomplete as well; the datastructure_id of the make_data_store table could use a generic description like Data structure ID..

2. The client folder can be moved out of the make folder to make sure that we keep the table code separate from the client connection code. Please take a look at https://github.com/turbot/steampipe-plugin-turbot for more information.

3. Is there a specific reason for defining separate logger functions? I think we could easily use the format that we follow for other tables. You could refer https://github.com/turbot/steampipe-plugin-pagerduty/blob/main/pagerduty/table_pagerduty_incident.go#L196 for more information. This will help us in maintaining consistency across all the plugins.

4. Could you please sort the tables in https://github.com/marekjalovec/steampipe-plugin-make/blob/main/make/plugin.go#L28-L39. We follow this format across all the plugins.

5. config/make.spc file contains great content 👍.

6. The queries in the table docs look great, but they are light in my opinion. It would be great if you could add 2-3 more example queries per table. Additionally, could you also format the queries using the SQL formatter ? Apologies that this requirement is not properly highlighted in the release checklist.

7. Could you please remove the query outputs from the table docs? As per our current structure, we do not include query outputs in our table docs 👍. Also, I feel we do not need separate headers for highlighting the Key Columns and Caveats. These descriptions can be included in the table description section of the docs as well.

8. Does the SDK support environment variables? If yes, I think we should add support for them in the plugin code and mention them explicitly in the docs as well.

9. All the tables are missing logging information when the API client function is invoked. Could you please add that across all the tables as per the format in https://github.com/turbot/steampipe-plugin-pagerduty/blob/main/pagerduty/table_pagerduty_incident.go#L194-L198

10. Is there a specific need for dev.sh script? Can we remove it?

11. Could you please update the .gitignore file to include the extensions mentioned in https://github.com/turbot/steampipe-plugin-pagerduty/blob/main/.gitignore

12. The plugin seems to be missing the .goreleaser.yml file. Could you please add that in as well 👍 .

13. The go.mod file is using 4.1.9 Steampipe Plugin SDK version. Could we please update it to use 5.0.2 SDK version.

Overall the plugin looks great 👍. Amazing to see how you made sure that pagination and rate limits are handled properly 👍.

Please let us know if you have questions or if you need a discussion on the above feedback. Happy to help 😄 .