Skip to content

Remove deprecated app module aliases.#15639

Merged
seanbudd merged 2 commits into
nvaccess:masterfrom
lukaszgo1:I15618
Oct 18, 2023
Merged

Remove deprecated app module aliases.#15639
seanbudd merged 2 commits into
nvaccess:masterfrom
lukaszgo1:I15618

Conversation

@lukaszgo1

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #15618

Summary of the issue:

PR #13366 changed a way in which NVDA maps an app module to a given executable. In the process several old app modules were marked as deprecated (they show warnings on import). This is pretty noticeable when building developer documentation scons devDocs.

Description of user facing changes

None

Description of development approach

Deprecated app modules and supporting functions from appModuleHandler were removed. The app Module for Azardi which cannot be imported by Sphinx was renamed, and mapped to Azardi's binary.

Testing strategy:

Ensured that app module for Azardi is loaded. Ensured that warnings when building developer documentation are gone.

Known issues with pull request:

None known

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.

@lukaszgo1 lukaszgo1 requested a review from a team as a code owner October 17, 2023 11:56
@lukaszgo1 lukaszgo1 requested a review from seanbudd October 17, 2023 11:56
@lukaszgo1 lukaszgo1 changed the title Remove deprecated app modle aliases. Remove deprecated app module aliases. Oct 17, 2023
@CyrilleB79

Copy link
Copy Markdown
Contributor

In the future, if there are appModules to rename, does it mean that it will not be allowed anymore to write deprecation code to map the old name to the new appModule? Or otherwise we would reintroduce #15618?

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

In the future, if there are appModules to rename, does it mean that it will not be allowed anymore to write deprecation code to map the old name to the new appModule? Or otherwise we would reintroduce #15618?

If name of old app module would be valid (i.e. would not contain characters such as + or .), then it should be possible to show deprecation warning on import only when not building developer documentation. DO you have a particular rename in mint?

@CyrilleB79

Copy link
Copy Markdown
Contributor

No I do not have any appModule in mind.
I was just wondering if there was a risk that the issue comes back again in the future, and if (and how) we can mitigate it.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

As I mentioned in #12971 we probably should enable building developer documentation in CI, and fail the build on any newly introduced warnings.

@seanbudd seanbudd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @lukaszgo1

Comment thread user_docs/en/changes.t2t Outdated
@seanbudd seanbudd merged commit 56f60b8 into nvaccess:master Oct 18, 2023
@nvaccessAuto nvaccessAuto added this to the 2024.1 milestone Oct 18, 2023
@lukaszgo1 lukaszgo1 deleted the I15618 branch October 18, 2023 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecated app module aliases should not cause warnings when building developer documentation.

4 participants