Skip to content

create Enums for audioDucking and minor lint#12926

Merged
seanbudd merged 6 commits into
masterfrom
audioducking-refactor
Oct 20, 2021
Merged

create Enums for audioDucking and minor lint#12926
seanbudd merged 6 commits into
masterfrom
audioducking-refactor

Conversation

@seanbudd

@seanbudd seanbudd commented Oct 13, 2021

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

While debugging #12913, I created some Enums to help with logging and understanding the code.
This is not a focused refactor, and as such, more work could be done to improve the audioDucking API.

Description of how this pull request fixes the issue:

  • Implements IntEnums for constants
  • Add some comments
  • Clean up imports

Testing strategy:

with a try build of this PR:

  • Test the audio ducking menu
  • Test the input gesture(s) to trigger audioducking
  • Test audio ducking as a feature

Known issues with pull request:

Note, this is an API breaking change and should not be merged until 2022.1.
I have just created this PR as the work should not be lost, but adding backwards compatibility is not worth prioritising.

Change log entries:

See diff in changes.t2t

Code Review Checklist:

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

@seanbudd seanbudd added this to the 2022.1 milestone Oct 13, 2021
@seanbudd seanbudd marked this pull request as ready for review October 19, 2021 22:55
@seanbudd seanbudd requested a review from a team as a code owner October 19, 2021 22:55

@michaelDCurran michaelDCurran 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.

Looks good other than the query re _displayStringLabels

Comment thread source/gui/settingsDialogs.py Outdated
self.duckingList = settingsSizerHelper.addLabeledControl(
duckingListLabelText,
wx.Choice,
choices=audioDucking.AudioDuckingMode._displayStringLabels.values()

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.

does this really work?
I think that _displayStringLabels is an instance property, and therefore can not be accessed on the class, rather it can only be accessed on the instance (one of the enum values).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, this doesn't work. I've tested this PR using a try build now, and fixed how the choices are generated

@seanbudd seanbudd merged commit 0c09fff into master Oct 20, 2021
@seanbudd seanbudd deleted the audioducking-refactor branch October 20, 2021 06:49
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.

2 participants