Skip to content
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

Docs: Update plugin.json doc as version is not required #992

Merged
merged 3 commits into from
Jul 15, 2024

Conversation

jackw
Copy link
Collaborator

@jackw jackw commented Jul 5, 2024

What this PR does / why we need it:

This PR updates the plugin.json doc so the version property of dependencies as it is neither required nor should it use semver ranges. See the accompanying plugin.schema PR here.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@jackw jackw added type/docs Changes only affect the documentation no-changelog Don't include in changelog and version calculations labels Jul 5, 2024
@jackw jackw self-assigned this Jul 5, 2024
Copy link

github-actions bot commented Jul 5, 2024

Hello! 👋 This repository uses Auto for releasing packages using PR labels.

✨ This PR can be merged. It will not be considered when calculating future versions of the npm packages and will not appear in the changelogs.

@jackw jackw marked this pull request as ready for review July 8, 2024 08:16
@jackw jackw requested a review from a team as a code owner July 8, 2024 08:16
@jackw jackw requested review from leventebalogh and removed request for a team July 8, 2024 08:16
Copy link
Member

@xnyo xnyo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@josmperez josmperez left a comment

Choose a reason for hiding this comment

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

LGTM.

| `id` | string | **Yes** | |
| `name` | string | **Yes** | |
| `type` | string | **Yes** | Possible values are: `app`, `datasource`, `panel`. |
| `version` | string | **No** | We suggest using [SemVer](https://semver.org/) as your plugin versioning scheme. This property should use a specific, pinned version and not a range. |
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add something along the lines "if no version specified, the latest supported version will be used"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah thanks @marefr . Added in c2858fb 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

as your plugin versioning scheme

I'm not sure if this is confusing as the versioning scheme of a plugin you declare a dependency on is not up to you

Copy link
Collaborator Author

@jackw jackw Jul 9, 2024

Choose a reason for hiding this comment

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

I don't think it's up to any plugin developer (original or dependent) as I'm pretty sure all our tooling and some checks within both grafana core and plugin codebases is geared around Semver and if they were to use anything else (such as date of release) things would go wrong somewhere. How about I reword it to:

This property should use a specific, pinned [SemVer](https://semver.org/) version and not a range. If no version is specified, it will resolve to the latest supported version.

and we update the info.version property blurb to read something like:

-Project version of this commit, e.g. `6.7.x`
+[SemVer](https://semver.org/) version of the plugin, e.g. `6.7.1`.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM 🚀

@jackw jackw merged commit 5c14120 into main Jul 15, 2024
11 checks passed
@jackw jackw deleted the jackw/docs-reference-version branch July 15, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-changelog Don't include in changelog and version calculations type/docs Changes only affect the documentation
Projects
Development

Successfully merging this pull request may close these issues.

8 participants