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

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

Closed
lukaszgo1 opened this issue Oct 12, 2023 · 1 comment · Fixed by #15639
Labels
audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Milestone

Comments

@lukaszgo1
Copy link
Contributor

Is your feature request related to a problem? Please describe.

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. The following two types of warnings are shown:

Importing appModules.digitaleditionspreview is deprecated, instead import appModules.digitaleditions.

WARNING: invalid signature for automodule ('appModules.azardi-2_0')
WARNING: don't know which module to import for autodocumenting 'appModules.azardi-2_0' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: invalid signature for automodule ('appModules.azuredatastudio-insiders')
WARNING: don't know which module to import for autodocumenting 'appModules.azuredatastudio-insiders' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)
WARNING: invalid signature for automodule ('appModules.code - insiders')
WARNING: don't know which module to import for autodocumenting 'appModules.code - insiders' (try placing a "module" or "currentmodule" directive in the document, or giving an explicit module name)

Describe the solution you'd like

Since these are deprecated aliases (with the exception of module for Azardi, see below) I suggest to remove them in the 2024.1 development cycle. We cannot just disable deprecated code when building developer documentation, since for most of these modules they cannot be imported by Sphinx (their names do not comply to the naming requirement of a Python module). Module for Azardi is a special case - it is not deprecated. However since we are in the backward compatibility breaking release, I suggest to just rename it into something like azardi20, and map it to the Azardi's binary.

Describe alternatives you've considered

If it is decided that keeping backward compat here is desired, it should be possible to write a custom importer which simulates presence of these modules at runtime. I'm willing to work on this, if that is the direction NV Access prefers.

Additional context

While I believe keeping backward compatibility is important, the deprecated code should not be getting in the way. I'm also not aware of any add-on which relies on presence of these deprecated app modules.

@seanbudd
Copy link
Member

Removing deprecated code makes sense.

@seanbudd seanbudd added p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers triaged Has been triaged, issue is waiting for implementation. labels Oct 16, 2023
seanbudd pushed a commit that referenced this issue Oct 18, 2023
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.
@nvaccessAuto nvaccessAuto modified the milestone: 2024.1 Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers p4 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants