Improve and refactor documentation #109

seanbudd · 2023-03-02T20:36:31Z

Closes #95

Issues fixed in this PR:

convertAddonManifest.py has been deleted as the issue form replaces this process. This python script was also an unintentional less polished duplicate of a script in the validation repo.
documentation is split into design/development docs, metadata schema information, and an add-on submission guide,
documentation is added on the latest and legacy end points
the submission process has been given a lot more detail, and largely rewritten
- this is to add information on all the fields in the template, derived from the schema file

Suggested review style: git diff master --color-moved

seanbudd · 2023-03-02T20:36:48Z

@CyrilleB79 could you review this?

CyrilleB79

Thanks @seanbudd for this rework of the doc.

I have made a lot of comments and proposals. Feel free to ignore what seems unsuitable to you.

Note: I have reviewed in details the first page and the submission doc. Regarding the design doc, I have read it quickly and found no obvious issue. But I have not read it in details as the two other pages.

README.md

CyrilleB79 · 2023-03-03T15:21:47Z

README.md

+[Read the submission guide](./docs/submitters/submissionGuide.md)
+
+### Design overview
+[Read the design overview](./docs/submitters/designOverview.md)

 ### About security 


Why not move this paragraph into designOverview.md?

I think this affects more than developers. I believe submitters, developers and other community contributors would find this information helpful.

docs/submitters/submissionGuide.md

CyrilleB79 · 2023-03-03T21:25:04Z

docs/submitters/submissionGuide.md

+The URL should remain valid indefinitely.
+GitHub release URLs are recommended.
+
+Example: `"https://github.com/nvaccess/addon-datastore/releases/download/v0.1.0/myAddon.nvda-addon"`


Real example here:

Suggested change

Example: `"https://github.com/nvaccess/addon-datastore/releases/download/v0.1.0/myAddon.nvda-addon"`

Example: `"https://github.com/nvdaes/placeMarkers/releases/download/24.0/placeMarkers-24.0.nvda-addon"`

what is the benefit of a real example here?

docs/submitters/submissionGuide.md

CyrilleB79 · 2023-03-03T21:32:02Z

docs/submitters/submissionGuide.md

+Refer to [addon-datastore-validation](https://github.com/nvaccess/addon-datastore-validation) for more information on automated checks.
+GitHub requires manual approval for the automated checks to run on an author's the first submission to the repository.
+1. If the checks pass, the PR should be merged automatically.
+However, a human review process may be required.


When is human review required? Are you speaking of first author's submission as already written above? Or something else?

I think it is more appropriate to keep this vague until the process has been tested and finalised.

Another example: changing an add-on file, deleting a file, changing the casing of an add-on ID.

josephsl · 2023-03-03T21:42:24Z

Hi, feel free to use either Add-on Updater or Windows App Essentials as an example. Thanks.

nvdaes · 2023-03-04T01:28:21Z

@CyrilleB79, I suggest that this PR is taken in consideration after two pending tasks:1. PR 17 of addon-datastore-validation repo is merged. sean mentioned, and I made changes accordingliy, that just valid api versions should be considered valid. You can see add-on-datastore-transform repo, NVDAapiversions.json file. three 0 version is rvalid, and the next available API version is 2019 3, so for min required and last tested versions, 2018 dot x wouldn"t be accepted and a valid api version is required.2. Automerge feature should be enabled before continuing with this. I am not sure, but according with I"m read, people will need write permission to use automerge feature. If this is true, forks shouldn"t be done. Instead, people should ask for writen access to this repo,though the master branch must be protected so nobody except nv access can write. I finally managed to enable automerge in one of my repos. If needed, someone may create a pull request to confirm if they can use this feature without write permissions, supossing that there is not a configuration missed by me.I suggest, if the submitters document is not related to the repo but to the whole system, add a note about requesting registration in the translation system. Mesar Hameed, original author of the addon guidelines and the first review process, who helped to host the first website and the translation system created by him, in the guidelines reflected characters not allowed like certain symbols, in addon repos, I think.About add-on version names, that is, the version that appears in the manifest, from which version number is built, I would suggest simply: version can be a two or three part string separated by dots, and each part can contain just numbers. Valid examples are 1 dot 0 dot 1 or 2023 dot cero three dot 11, that is, semver like or date like scheemas. Please don"t include non numeric characters in version names.

nvdaes · 2023-03-04T02:43:29Z

One more consideration: Previously I was investigating, long time ago, about the possibility of creating pull request to the addons-datastore repo generating json metadata files using issues with a form. At this moment this was delayed and explained why, but this was time ago. Maybe revisited in case this makes easy automerge? I think that automerge can be enabled with write permissions, but I don"t know if this would be needed with issues if the Pull Request action is triggered inside the repo providing a personal access token stored as a secret. The problem that i remember was that issues weren"t closed automatically. I thing that I"ll investigate again in my own repo to see if checks are performed. I"ll comment here my results since they maybe relevant in terms of when this PR is merged since it"s related to documentation.

CyrilleB79 · 2023-03-04T22:52:59Z

@nvdaes IMO the current PR should be merged as soon as possible since people are already expected to use this documentation to submit their add-ons. And this PR is already a great clarification for users (add-on authors).

If all cannot yet be described clearly because there are still unknown inputs or decisions to be made (e.g. regarding versioning), a subsequent PR can be opened for further modification of the documentation.

nvdaes · 2023-03-04T23:53:26Z

Your argument makes sense for me. Ok for my part.Enviado desde mi iPhoneEl 4 mar 2023, a las 23:53, Cyrille Bougot ***@***.***> escribió: @nvdaes IMO the current PR should be merged as soon as possible since people are already expected to use this documentation to submit their add-ons. And this PR is already a great clarification for users (add-on authors). If all cannot yet be described clearly because there are still unknown inputs or decisions to be made (e.g. regarding versioning), a subsequent PR can be opened for further modification of the documentation. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

seanbudd · 2023-03-06T01:32:51Z

README.md

+[Read the submission guide](./docs/submitters/submissionGuide.md)
+
+### Design overview
+[Read the design overview](./docs/submitters/designOverview.md)

 ### About security 


I think this affects more than developers. I believe submitters, developers and other community contributors would find this information helpful.

seanbudd · 2023-03-06T01:35:05Z

README.md


 ### About security 
 Ensuring that an add-on is safe to run is a difficult challenge that isn't addressed here.
 However, the metadata for a new submission (add-on release) can be confirmed to match its manifest
 description.
-Additionally, add-on file integrity can be enforced via a Sha256 checksum.
+Additionally, add-on file integrity can be enforced via a SHA256 checksum.
 The checksum allows NVDA to ensure that add-on releases are immutable.

 ### Human review process / code audit


I think this information is more relevant to other parties than submitters: including developers, add-on reviewers, and other community members.

seanbudd · 2023-03-06T01:36:48Z

docs/submitters/submissionGuide.md

+### Manual file creation
+On a new branch, copy the `_template_addon_release.json` file. 
+- Rename and move the file to `addons/<addonID>/<version>.json`
+- `<addonID>` is the ID of the add-on. This should match the `name` field in the add-on manifest, e.g. "speechPlayer"


I think a fictional example is better, perhaps something that never existed is better.

Suggested change

- `<addonID>` is the ID of the add-on. This should match the `name` field in the add-on manifest, e.g. "speechPlayer"

- `<addonID>` is the ID of the add-on. This should match the `name` field in the add-on manifest, e.g. "speechMagic"

docs/submitters/submissionGuide.md

seanbudd · 2023-03-06T02:02:23Z

docs/submitters/submissionGuide.md

+The URL should remain valid indefinitely.
+GitHub release URLs are recommended.
+
+Example: `"https://github.com/nvaccess/addon-datastore/releases/download/v0.1.0/myAddon.nvda-addon"`


what is the benefit of a real example here?

docs/submitters/submissionGuide.md

seanbudd · 2023-03-06T02:06:43Z

docs/submitters/submissionGuide.md

+Optional.
+If the addon has a homepage where users can get more information about the addon, you can specify it here.
+
+Example: `"https://github.com/nvaccess/addon-datastore"`


Could you explain the benefit of an real example here?
I think pointing to one might imply a convention on homepage design, which is definitely not the case.
Homepages can be anything e.g. GitHub readmes or personal websites.

seanbudd · 2023-03-07T20:52:24Z

@nvdaes I've updated this to include the new issue form submission.

nvdaes · 2023-03-07T21:04:33Z

I've seen the explanation about issues and seems good.

seanbudd · 2023-03-07T21:17:36Z

@CyrilleB79 - I am going to merge this as it greatly improves the current state of documentation, and there is a time pressure on getting these out.
Feel free to respond to this review further and a subsequent PR will be opened, or open a new issue/PR yourself suggesting changes.

seanbudd added 3 commits March 2, 2023 12:29

Improve and refactor documentation

b3c33b9

fix md

ff041f5

fix md

6e0a318

seanbudd requested a review from michaelDCurran March 2, 2023 20:36

seanbudd added 2 commits March 2, 2023 12:43

fix up links

dd2ac8a

remove apostrophes

81eed31

CyrilleB79 reviewed Mar 3, 2023

View reviewed changes

seanbudd mentioned this pull request Mar 6, 2023

Use GitHub actions to check the SHAs and validity of URLs #112

Closed

Apply suggestions from code review

32c8f2b

Co-authored-by: Cyrille Bougot <cyrille.bougot2@laposte.net>

seanbudd commented Mar 6, 2023

View reviewed changes

seanbudd added 2 commits March 7, 2023 09:39

Apply suggestions from code review

a6c146d

improve docs on channels

9905a3b

seanbudd mentioned this pull request Mar 6, 2023

Only show dev channel add-ons for pre-release NVDA nvaccess/nvda#14693

Closed

6 tasks

seanbudd added 2 commits March 7, 2023 12:24

Merge remote-tracking branch 'origin/master' into improveDocumentation

799d090

Use issue form for submission

f4bd5ed

fix up instructions

c5ccd92

seanbudd merged commit 1cadb63 into master Mar 7, 2023

seanbudd deleted the improveDocumentation branch March 7, 2023 21:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve and refactor documentation #109

Improve and refactor documentation #109

seanbudd commented Mar 2, 2023 •

edited

Loading

seanbudd commented Mar 2, 2023

CyrilleB79 left a comment •

edited

Loading

CyrilleB79 Mar 3, 2023

seanbudd Mar 6, 2023

CyrilleB79 Mar 3, 2023

seanbudd Mar 6, 2023

CyrilleB79 Mar 3, 2023

seanbudd Mar 6, 2023

seanbudd Mar 6, 2023

josephsl commented Mar 3, 2023 via email

nvdaes commented Mar 4, 2023 via email •

edited by seanbudd

Loading

nvdaes commented Mar 4, 2023 via email •

edited by seanbudd

Loading

CyrilleB79 commented Mar 4, 2023

nvdaes commented Mar 4, 2023 via email

seanbudd Mar 6, 2023

seanbudd Mar 6, 2023

seanbudd Mar 6, 2023

seanbudd Mar 6, 2023

seanbudd Mar 6, 2023

seanbudd commented Mar 7, 2023

nvdaes commented Mar 7, 2023

seanbudd commented Mar 7, 2023

	Example: `"https://github.com/nvaccess/addon-datastore/releases/download/v0.1.0/myAddon.nvda-addon"`
	Example: `"https://github.com/nvdaes/placeMarkers/releases/download/24.0/placeMarkers-24.0.nvda-addon"`

	- `<addonID>` is the ID of the add-on. This should match the `name` field in the add-on manifest, e.g. "speechPlayer"
	- `<addonID>` is the ID of the add-on. This should match the `name` field in the add-on manifest, e.g. "speechMagic"

Improve and refactor documentation #109

Improve and refactor documentation #109

Conversation

seanbudd commented Mar 2, 2023 • edited Loading

seanbudd commented Mar 2, 2023

CyrilleB79 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephsl commented Mar 3, 2023 via email

nvdaes commented Mar 4, 2023 via email • edited by seanbudd Loading

nvdaes commented Mar 4, 2023 via email • edited by seanbudd Loading

CyrilleB79 commented Mar 4, 2023

nvdaes commented Mar 4, 2023 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanbudd commented Mar 7, 2023

nvdaes commented Mar 7, 2023

seanbudd commented Mar 7, 2023

seanbudd commented Mar 2, 2023 •

edited

Loading

CyrilleB79 left a comment •

edited

Loading

nvdaes commented Mar 4, 2023 via email •

edited by seanbudd

Loading

nvdaes commented Mar 4, 2023 via email •

edited by seanbudd

Loading