Skip to content

move/remove imports to prevent circular dependencies#12404

Merged
seanbudd merged 6 commits into
betafrom
prevent-circular-dep
May 18, 2021
Merged

move/remove imports to prevent circular dependencies#12404
seanbudd merged 6 commits into
betafrom
prevent-circular-dep

Conversation

@seanbudd

@seanbudd seanbudd commented May 12, 2021

Copy link
Copy Markdown
Member

Link to issue number:

Fixes #12399

Summary of the issue:

NVDA fails to uninstall addons which import gui due to a circular dependency going from speech.py -> sayAll.py

While the refactor of speech/sayAll #12251 was designed to make fixing this problem simpler, it did not actually fix this issue.

Description of how this pull request fixes the issue:

  • makes sayAll no longer dependent on speech by injecting the dependencies on initialization
  • removes unused imports
  • moves imports to lazy load

Testing strategy:

Manual testing that #12399 is fixed

Known issues with pull request:

none

Change log entries:

None needed

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

@seanbudd seanbudd added this to the 2021.1 milestone May 12, 2021
@seanbudd seanbudd self-assigned this May 12, 2021
@seanbudd seanbudd requested a review from a team as a code owner May 12, 2021 04:31
@seanbudd seanbudd requested a review from michaelDCurran May 12, 2021 04:31
@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 73fdb7b609

@seanbudd seanbudd requested a review from feerrenrut May 12, 2021 06:31
Comment thread source/documentBase.py Outdated
Comment thread source/core.py Outdated
@seanbudd seanbudd requested a review from feerrenrut May 14, 2021 00:46
Comment thread source/speech/__init__.py Outdated
Comment thread source/documentBase.py Outdated
Comment thread source/speech/__init__.py
@seanbudd seanbudd requested a review from feerrenrut May 17, 2021 03:25
feerrenrut
feerrenrut previously approved these changes May 17, 2021

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please update the target to beta, and merge. We'll need to do another beta to master merge soon.

@seanbudd seanbudd changed the base branch from master to beta May 17, 2021 06:19
@seanbudd seanbudd dismissed feerrenrut’s stale review May 17, 2021 06:19

The base branch was changed.

@seanbudd seanbudd requested a review from feerrenrut May 17, 2021 06:19

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The diff includes unrelated stuff from master (Eg the CLDR changes). You'll need to rebase.

@seanbudd seanbudd force-pushed the prevent-circular-dep branch 2 times, most recently from 721437d to 87ae1ea Compare May 17, 2021 09:01
@seanbudd seanbudd force-pushed the prevent-circular-dep branch from 2fb0ab2 to e0536d9 Compare May 17, 2021 09:02
@seanbudd seanbudd requested a review from feerrenrut May 17, 2021 09:03

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @seanbudd

@seanbudd

Copy link
Copy Markdown
Member Author

@feerrenrut I'm unable to merge this

@seanbudd seanbudd merged commit b6997e3 into beta May 18, 2021
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.

NVDA crashes when uninstalling add-ons importing gui at the module level in installTasks.

3 participants