Skip to content

Remove unnecessary SchedulingViewControllerProtocol - #19708

Merged
mokagio merged 1 commit into
trunkfrom
mokagio/remove-schedulingviewcontrollerprotocol
Dec 2, 2022
Merged

Remove unnecessary SchedulingViewControllerProtocol#19708
mokagio merged 1 commit into
trunkfrom
mokagio/remove-schedulingviewcontrollerprotocol

Conversation

@mokagio

@mokagio mokagio commented Dec 2, 2022

Copy link
Copy Markdown
Contributor

This protocol used to be a facade between SchedulingDatePickerViewController and SchedulingCalendarViewController but the latter has been removed in a60d3d9 during the iOS 13 Widgets removal so we can remove the abstraction.

Testing

If CI builds, we're good to go.

Regression Notes

  1. Potential unintended areas of impact – N.A.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes. N.A.
  • I have considered adding accessibility improvements for my changes. N.A.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

This protocol used to be a facade between `SchedulingDatePickerViewController` and
`SchedulingCalendarViewController` but the latter has been removed in
a60d3d9 so we can remove the
abstraction.
@mokagio
mokagio enabled auto-merge December 2, 2022 04:54
@mokagio mokagio self-assigned this Dec 2, 2022
@mokagio mokagio added this to the 21.4 milestone Dec 2, 2022
@wpmobilebot

Copy link
Copy Markdown
Contributor
You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19708-20df412 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot

Copy link
Copy Markdown
Contributor
You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19708-20df412 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

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

Code looks good. I've not tested it though as I don't have a WPiOS dev env set up right now: I added this on a maintenance rotation. Since it's building, I expect it's fine.

@@ -26,7 +26,7 @@ class DateCoordinator {
// MARK: - Date Picker

@available(iOS, introduced: 14.0)

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.

Perhaps this can be removed as well? I can think of arguments for or against, so either way is good with me, but just checking whether you've considered it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I should have mentioned that I left that for #19709 . Thanks!

@mokagio
mokagio merged commit 4ae5cd6 into trunk Dec 2, 2022
@mokagio
mokagio deleted the mokagio/remove-schedulingviewcontrollerprotocol branch December 2, 2022 08:18
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.

3 participants