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

[MM-232] Update plugin with respect to phase 1 upgrades #111

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ayusht2810
Copy link

Summary

Updated plugin with respect to phase 1 upgrades

Ticket Link

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 22.52%. Comparing base (bbf7349) to head (d2b4575).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
- Coverage   23.12%   22.52%   -0.60%     
==========================================
  Files           6        6              
  Lines         506      506              
==========================================
- Hits          117      114       -3     
- Misses        372      374       +2     
- Partials       17       18       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mickmister mickmister requested a review from hanzei March 14, 2024 04:48
README.md Outdated
Comment on lines 82 to 89
### Releasing new versions

The version of a plugin is determined at compile time, automatically populating a `version` field in the [plugin manifest](plugin.json):
* If the current commit matches a tag, the version will match after stripping any leading `v`, e.g. `1.3.1`.
* Otherwise, the version will combine the nearest tag with `git rev-parse --short HEAD`, e.g. `1.3.1+d06e53e1`.
* If there is no version tag, an empty version will be combined with the short hash, e.g. `0.0.0+76081421`.

To disable this behaviour, manually populate and maintain the `version` field.
Copy link
Contributor

Choose a reason for hiding this comment

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

The README should make clear what this plugin is using. As I pointed out in another PR, I still think that version bump PRs are helpful, and we should keep them for Community-Supported plugins.

Choose a reason for hiding this comment

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

@hanzei Not sure what we need to do here. Can you please provide some details?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. The last sentence

To disable this behaviour, manually populate and maintain the version field.

suggests that there are two ways to release a plugin: either with the version field or without it. The README should make it clear which way is used in this plugin and should not leave room for interpretation.

Choose a reason for hiding this comment

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

@hanzei Updated the readme for better clarity, please have a look

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Mar 15, 2024
Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

I don't think we should spend more time reviewing this plugin. Approving

@raghavaggarwal2308 raghavaggarwal2308 added the 3: QA Review Requires review by a QA tester label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants