Skip to content

Conversation

@mokagio
Copy link
Contributor

@mokagio mokagio commented Dec 6, 2022

Just a couple of easy wins I noticed. See code for more details...

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.

@peril-wordpress-mobile
Copy link

Warnings
⚠️ PR is not assigned to a milestone.
⚠️ PR is missing at least one label.

Generated by 🚫 dangerJS

static func getTags() -> [String] {

let context = ContextManager.sharedInstance().mainContext
let blogService = BlogService(managedObjectContext: context)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crazytonyli I think this was a leftover from the various iterations on the API used to fetch resources, right?

I checked the implementation and couldn't find a reason to init the service without using it. But I might have missed something...

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it must be, but should be safe to remove though. Thanks!

let currentLightIcons = allIcons.filter({ $0.isLegacy == false && $0.isBordered == true })
.sorted(by: sortWithPriority(toItemsWithPrefix: AppIcon.defaultIconName))
let legacyIcons = {
var icons = allIcons.filter({ $0.isLegacy == true })
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The warning itself was on this being a var that was never mutated. While I was at it, I also adopted guard for the early-return.

@mokagio mokagio requested a review from crazytonyli December 6, 2022 16:05
@mokagio mokagio self-assigned this Dec 6, 2022
@mokagio mokagio added this to the 21.4 milestone Dec 6, 2022
@wpmobilebot
Copy link
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 pr19746-bdacc59 on your iPhone

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

@wpmobilebot
Copy link
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 pr19746-bdacc59 on your iPhone

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

@mokagio mokagio merged commit 9f9a762 into trunk Dec 7, 2022
@mokagio mokagio deleted the mokagio/address-some-warnings branch December 7, 2022 04:20
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.

4 participants