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

Make add-on store base end point part of the public API #15893

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented Dec 8, 2023

Link to issue number:

Relates to #14974

Summary of the issue:

Add-on authors wish to have a stable API so that they can change the base URL of the add-on store.
This allows add-ons to use a mirror for the add-on store.

Description of user facing changes

None

Description of development approach

Change BASE_URL for the add-on store to be public

Testing strategy:

Confirm with add-on authors that changes are sufficient

Known issues with pull request:

None

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@seanbudd seanbudd requested a review from a team as a code owner December 8, 2023 04:50
@seanbudd
Copy link
Member Author

seanbudd commented Dec 8, 2023

@cary-rowen can you confirm that these changes are sufficient to create a stable API for your mirroring add-on?

@seanbudd seanbudd added this to the 2024.1 milestone Dec 8, 2023
@LeonarddeR
Copy link
Collaborator

Isn't it possible to provide a default host that is also available from China? Why not use github pages, for example?

@seanbudd
Copy link
Member Author

seanbudd commented Dec 8, 2023

@LeonarddeR - I would consider that out of scope and somewhat covered by #14974
I'm not sure if GitHub pages supports the functionality we need and use on the server, it would require a separate investigation

@lukaszgo1
Copy link
Contributor

Note that Add-on Updater allows user to select one of the supported mirrors from its GUI, which IMHO is a much nicer UX. While I don't think we should promote any particular mirrors in NVDA as such, offering ability to add an URL from the config dialog seems much better than just making API for add-ons more stable.

@cary-rowen
Copy link
Contributor

Hi @seanbudd,
I haven't tested this yet, but I think that this might not be the best solution, In China, in addition to the add-on store mirror of NVDA, we have created an updated mirror of NVDA.
The ideal effect may be that users can smoothly use the NVDA add-on store and automatic updates without installing mirror add-on.
Because users cannot access the add-on store until the mirror add-on is installed.

@seanbudd
Copy link
Member Author

Hi @cary-rowen, as mentioned earlier, that is out of scope of what we are trying to do here, the issue exists to track that further work. Now that the add-on API is considered stable we can ensure the API your add-on needs is stable.

@cary-rowen
Copy link
Contributor

If only exposing the add-on store API is considered then this can be considered stable.

@seanbudd seanbudd merged commit 7db27de into master Dec 12, 2023
1 check passed
@seanbudd seanbudd deleted the makeAddonStoreEndPointPublic branch December 12, 2023 01:39
Adriani90 pushed a commit to Adriani90/nvda that referenced this pull request Mar 13, 2024
Relates to nvaccess#14974

Summary of the issue:
Add-on authors wish to have a stable API so that they can change the base URL of the add-on store.
This allows add-ons to use a mirror for the add-on store.

Description of user facing changes
None

Description of development approach
Change BASE_URL for the add-on store to be public
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants