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.

Sign up for GitHub

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

Jump to bottom

move/remove imports to prevent circular dependencies #12404

Merged
merged 6 commits into from
May 18, 2021
Merged

move/remove imports to prevent circular dependencies #12404

merged 6 commits into from
May 18, 2021

Conversation

seanbudd
Copy link
Member

@seanbudd seanbudd commented May 12, 2021 •
edited
Loading

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
@AppVeyorBot
Copy link

See test results for failed build of commit 73fdb7b609

@seanbudd seanbudd requested a review from feerrenrut May 12, 2021 06:31
feerrenrut
feerrenrut suggested changes May 13, 2021
View reviewed changes
source/documentBase.py Outdated Show resolved Hide resolved
source/core.py Outdated Show resolved Hide resolved
@seanbudd seanbudd requested a review from feerrenrut May 14, 2021 00:46
seanbudd
seanbudd commented May 14, 2021
View reviewed changes
source/speech/__init__.py Outdated Show resolved Hide resolved
feerrenrut
feerrenrut reviewed May 14, 2021
View reviewed changes
source/documentBase.py Outdated Show resolved Hide resolved
@seanbudd seanbudd requested a review from feerrenrut May 17, 2021 03:25
feerrenrut
feerrenrut previously approved these changes May 17, 2021
View reviewed changes
Copy link
Contributor

@feerrenrut feerrenrut left a comment

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 suggested changes May 17, 2021
View reviewed changes
Copy link
Contributor

@feerrenrut feerrenrut left a comment

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 requested a review from feerrenrut May 17, 2021 09:03
feerrenrut
feerrenrut approved these changes May 17, 2021
View reviewed changes
Copy link
Contributor

@feerrenrut feerrenrut left a comment

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
Member Author

@feerrenrut I'm unable to merge this

@seanbudd seanbudd merged commit b6997e3 into beta May 18, 2021
@seanbudd seanbudd deleted the prevent-circular-dep branch May 18, 2021 23:18
@seanbudd seanbudd mentioned this pull request May 21, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feerrenrut feerrenrut feerrenrut approved these changes

@michaelDCurran michaelDCurran Awaiting requested review from michaelDCurran

Assignees

@seanbudd seanbudd

Labels
None yet
Projects
None yet
Milestone
2021.1
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
@seanbudd @AppVeyorBot @feerrenrut