Update Add-on Manifest spec descriptions #14754
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See test results for failed build of commit 4d8dd46cd5 |
|
Does this men then that users will no longer be able to edit manifests of
installed add ons where they will work if edited?
Brian
…--
***@***.***
Sent via blueyonder.(Virgin media)
Please address personal E-mail to:-
***@***.***, putting 'Brian Gaff'
in the display name field.
----- Original Message -----
From: "Sean Budd" ***@***.***>
To: "nvaccess/nvda" ***@***.***>
Cc: "Subscribed" ***@***.***>
Sent: Tuesday, March 28, 2023 5:55 AM
Subject: [nvaccess/nvda] Update Add-on Manifest spec descriptions (PR
#14754)
<!-- Please read and fill in the following template, for an explanation
of the sections see:
https://github.com/nvaccess/nvda/blob/master/devDocs/githubPullRequestTemplateExplanationAndExamples.md
Please also note that the NVDA project has a Citizen and Contributor Code
of Conduct which can be found at
https://github.com/nvaccess/nvda/blob/master/CODE_OF_CONDUCT.md. NV Access
expects that all contributors and other community members read and abide
by the rules set out in this document while participating or contributing
to this project. This includes creating or commenting on issues and pull
requests.
Please initially open PRs as a draft. See
https://github.com/nvaccess/nvda/wiki/Contributing
-->
### Link to issue number:
None
### Summary of the issue:
The add-on datastore has certain expectations of an add-on manifest.
These expectations are not required to work with NVDA, but require
manifests to updated to be hosted on the add-on datastore.
We may want to make these changes in a future API breaking release, and
add validation to enforce them.
### Description of user facing changes
None
### Description of development approach
Updates the add-on manifest spec to match the requirements of the add-on
store better.
We may want to add validation for these requirements in a future API
breaking release.
### Testing strategy:
None
### Known issues with pull request:
None
### Change log entries:
For Developers
TODO
### Code Review Checklist:
<!--
This checklist is a reminder of things commonly forgotten in a new PR.
Authors, please do a self-review of this pull-request.
Check items to confirm you have thought about the relevance of the item.
Where items are missing (eg unit / system tests), please explain in the
PR.
To check an item `- [ ]` becomes `- [x]`, note spacing.
You can also check the checkboxes after the PR is created.
A detailed explanation of this checklist is available here:
https://github.com/nvaccess/nvda/blob/master/devDocs/githubPullRequestTemplateExplanationAndExamples.md#code-review-checklist
-->
- [x] Pull Request description:
- description is up to date
- change log entries
- [x] Testing:
- Unit tests
- System (end to end) tests
- Manual testing
- [x] API is compatible with existing add-ons.
- [ ] Documentation:
- User Documentation
- Developer / Technical Documentation
- Context sensitive help for GUI changes
- [ ] UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English
- [ ] Security precautions taken.
You can view, comment on, or merge this pull request online at:
#14754
-- Commit Summary --
* Update Add-on Manifest spec descriptions
-- File Changes --
M source/addonHandler/__init__.py (12)
-- Patch Links --
https://github.com/nvaccess/nvda/pull/14754.patch
https://github.com/nvaccess/nvda/pull/14754.diff
--
Reply to this email directly or view it on GitHub:
#14754
You are receiving this because you are subscribed to this thread.
Message ID: ***@***.***>
|
CyrilleB79
reviewed
Mar 28, 2023
source/addonHandler/__init__.py
Outdated
| @@ -737,7 +739,8 @@ class AddonManifest(ConfigObj): | |||
| # Name of the author or entity that created the add-on | |||
| author = string() | |||
|
|
|||
| # Version of the add-on. Should preferably in some standard format such as x.y.z | |||
| # Version of the add-on. | |||
| # Should be in <major>.<minor>.<patch> format. | |||
There was a problem hiding this comment.
Does it mean that in the future, X.Y.Z will be mandatory?
For now, I have not dealt with beta versions. However, I was hoping to use version names such as "4.3beta1". "4.3.1" does not indicate clearly to the user that it is a beta version. Having a version name indicating clearly which version is a beta and which not in the add-on manager (or in the future add-on store) would be a plus IMO.
Same for dev versions even if I personally do not plan to release dev versions of my add-ons.
There was a problem hiding this comment.
submissions with these formats may not be valid for the add-on store. The add-on store requires a major.minor.pitch version scheme.
This current PR just proposes encouraging authors to follow the standards for the add-on store when making a manifest, not force any mandatory changes.
It's still an open question whether or not we add validation enforcing major.minor.patch in all add-on manifests, not just submissions to the add-on store.
This would not be until a breaking release of course.
There was a problem hiding this comment.
Suggested change
| # Should be in <major>.<minor>.<patch> format. | |
| # Suggested convention is <major>.<minor>.<patch> format. |
no, this has nothing to do with this change |
seanbudd
commented
Mar 29, 2023
See test results for failed build of commit 19b969e7bf |
michaelDCurran
approved these changes
Apr 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
None
Summary of the issue:
The add-on datastore has certain expectations of an add-on manifest.
These expectations are not required to work with NVDA, but require manifests to be updated to be hosted on the add-on datastore.
We may want to make these changes in a future API breaking release, and add validation to enforce them.
Description of user facing changes
None
Description of development approach
Updates the add-on manifest spec to match the requirements of the add-on store better.
We may want to add validation for these requirements in a future API breaking release.
Testing strategy:
None
Known issues with pull request:
None
Change log entries:
For Developers
TODO:
add description of changes
when merging update add-on authors via add-on API mailing list.
Code Review Checklist: