Skip to content

fix: Set plugin metadata#14698

Closed
hermanschaaf wants to merge 5 commits intomainfrom
set-plugin-metadata
Closed

fix: Set plugin metadata#14698
hermanschaaf wants to merge 5 commits intomainfrom
set-plugin-metadata

Conversation

@hermanschaaf
Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf commented Oct 19, 2023

This sets plugin metadata on all the CloudQuery-maintained plugins, following an update to plugin-sdk v4.16.0, which requires this metadata to be present for the package command to succeed.

@hermanschaaf hermanschaaf requested a review from a team October 19, 2023 09:15
hermanschaaf
hermanschaaf commented Oct 19, 2023
View reviewed changes
plugins/source/firestore/resources/plugin/plugin.go
"github.com/cloudquery/plugin-sdk/v4/plugin"
)

var version = "development"
Copy link
Copy Markdown
Contributor Author

@hermanschaaf hermanschaaf Oct 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that version was unexported here was actually a bug, as even official versions of the plugin right now report their version to be development:

./plugin --version
firestore version development

@disq
Copy link
Copy Markdown
Member

disq commented Oct 19, 2023 •
edited
Loading

Couple of questions:

  • Why var when they are constants?
  • Why not use strings inside plugin init and define them as vars (or consts) when it's only in one place anyway, i.e. p := plugin.NewPlugin("azblob", internalPlugin.Version, client.New, plugin.WithKind("source"), plugin.WithTeam("cloudquery"))

@hermanschaaf hermanschaaf added automerge Automatically merge once required checks pass priority merge labels Oct 19, 2023
@hermanschaaf
Copy link
Copy Markdown
Contributor Author

hermanschaaf commented Oct 19, 2023

@disq Keeping them var because we may want to replace these values with ldflag in the future (like we already do for version), and keeping them grouped because I think it's nice to have this metadata visible/editable in one place, rather than in a function call.

@hermanschaaf
Copy link
Copy Markdown
Contributor Author

hermanschaaf commented Oct 19, 2023

Closing for now, I did a bad merge

@hermanschaaf hermanschaaf mentioned this pull request Oct 19, 2023
Merged
@disq disq deleted the set-plugin-metadata branch October 19, 2023 10:16
hermanschaaf added a commit that referenced this pull request Oct 19, 2023
@hermanschaaf @disq
@mnorbury @kodiakhq
fix: Set plugin metadata (#14715)
39935e2 
This sets plugin metadata on all the CloudQuery-maintained plugins,
following an update to plugin-sdk v4.16.0, which requires this metadata
to be present for the `package` command to succeed.

Second attempt of
[#14698](#14698)

---------

Co-authored-by: Kemal <223029+disq@users.noreply.github.com>
Co-authored-by: Martin Norbury <martin.norbury@gmail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
hydratim pushed a commit to hydratim/cloudquery that referenced this pull request Oct 20, 2023
@hermanschaaf @disq
@mnorbury @kodiakhq @hydratim
fix: Set plugin metadata (cloudquery#14715)
747cfbb 
This sets plugin metadata on all the CloudQuery-maintained plugins,
following an update to plugin-sdk v4.16.0, which requires this metadata
to be present for the `package` command to succeed.

Second attempt of
[cloudquery#14698](cloudquery#14698)

---------

Co-authored-by: Kemal <223029+disq@users.noreply.github.com>
Co-authored-by: Martin Norbury <martin.norbury@gmail.com>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@disq disq disq approved these changes

@yevgenypats yevgenypats Awaiting requested review from yevgenypats

@erezrokah erezrokah Awaiting requested review from erezrokah

Assignees

No one assigned

Labels

area/ci area/cli area/plugin/destination/azblob area/plugin/destination/bigquery area/plugin/destination/clickhouse area/plugin/destination/duckdb area/plugin/destination/elasticsearch area/plugin/destination/file area/plugin/destination/firehose area/plugin/destination/gcs area/plugin/destination/gremlin area/plugin/destination/kafka area/plugin/destination/meilisearch area/plugin/destination/mongodb area/plugin/destination/mssql area/plugin/destination/mysql area/plugin/destination/neo4j area/plugin/destination/postgresql area/plugin/destination/s3 area/plugin/destination/snowflake area/plugin/destination/sqlite area/plugin/source/alicloud area/plugin/source/aws area/plugin/source/awspricing area/plugin/source/azure area/plugin/source/azuredevops area/plugin/source/cloudflare area/plugin/source/datadog area/plugin/source/digitalocean area/plugin/source/facebookmarketing area/plugin/source/fastly area/plugin/source/firestore area/plugin/source/gcp area/plugin/source/github area/plugin/source/gitlab area/plugin/source/googleanalytics area/plugin/source/hackernews area/plugin/source/homebrew area/plugin/source/hubspot area/plugin/source/jira area/plugin/source/k8s area/plugin/source/notion area/plugin/source/okta area/plugin/source/oracle area/plugin/source/oracledb area/plugin/source/pagerduty area/plugin/source/salesforce area/plugin/source/shopify area/plugin/source/snyk area/plugin/source/source-mysql area/plugin/source/source-postgresql area/plugin/source/stripe area/plugin/source/terraform area/plugin/source/vault area/scaffold area/website automerge Automatically merge once required checks pass priority merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hermanschaaf @disq @cq-bot