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

Small refactor UnifiedPushHelper #6936

Closed
wants to merge 7 commits into from

Conversation

p1gp1g
Copy link
Contributor

@p1gp1g p1gp1g commented Aug 25, 2022

Type of change

  • Feature
  • Bugfix
  • Technical
  • Other : refactor

Content

Small refactor of UnifiedPushHelper : simplified and redundancies removed.

Motivation and context

As shown in #6890, before this PR (6890), the unregisterFirst parameter was only used when there was no distributor. And it is set to true only when it is removed first (registerInternal with force = true). So this parameter is useless.

Simplifying this Helper may avoid other issues like the dialog opened without the allowExternalUnifiedPushDistributors feature.

Screenshots / GIFs

Tests

  • Step 1
  • Step 2
  • Step ...

Tested devices

  • Physical
  • Emulator
  • OS version(s): 12

Checklist

ouchadam and others added 5 commits August 25, 2022 09:05
ATM, it uses the default fallback if cancelled
The forced unregistration always happens in register
function
@p1gp1g p1gp1g changed the title Misc/refactor uphelper Small refactor UnifiedPushHelper Aug 25, 2022
Signed-off-by: sim <git@sgougeon.fr>
@bmarty bmarty added the Z-Community-PR Issue is solved by a community member's PR label Aug 25, 2022
changelog.d/6936.misc Outdated Show resolved Hide resolved
Co-authored-by: Benoit Marty <benoit.marty@gmail.com>
@p1gp1g p1gp1g mentioned this pull request Sep 8, 2022
15 tasks
@p1gp1g
Copy link
Contributor Author

p1gp1g commented Sep 12, 2022

Merged with #7068

@p1gp1g p1gp1g closed this Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants