Skip to content

Detekt - Resolve/Suppress All Baseline Warnings - Long methods warnings_ MediaPickerActivity - #17355

Merged
ParaskP7 merged 2 commits into
wordpress-mobile:trunkfrom
Hoossayn:issue/17010_Detekt_Resolve_Suppress_All_Baseline_Warnings_long_method_media_picker
Oct 20, 2022
Merged

Detekt - Resolve/Suppress All Baseline Warnings - Long methods warnings_ MediaPickerActivity#17355
ParaskP7 merged 2 commits into
wordpress-mobile:trunkfrom
Hoossayn:issue/17010_Detekt_Resolve_Suppress_All_Baseline_Warnings_long_method_media_picker

Conversation

@Hoossayn

@Hoossayn Hoossayn commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

Parent: #17010

This PR resolved/suppress all complexity related LongMethod warnings for the WordPress module (see docs here):

1 x LongMethod (Resolve: 8594712 + f5cbb2e)
To test:

  • There is nothing much to test here.

  • Verifying that all the CI checks are successful should be enough (especially the detekt check).

  • However, if you really want to be thorough, you could smoke test the WordPress and/or Jetpack apps to verify that everything works as expected on every screen that relates to these changes.
    STEPS

  • Go to the menu tab

  • click on media option

  • click on the upload media button or the (+) button at the top right corner

  • select choose from device

  • select an image

Regression Notes

  1. Potential unintended areas of impact
    can't think of any

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    See To test section above.

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

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

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

👋 @Hoossayn !

Thank you so much for re-opening this PR! 🎉

I reviewed and tested this PR as per the description, along with triggering CI on it via a draft #17361 PR, which I just pushed on the main repo.


Everything LGTM, you did a great job in applying all my suggestion from the previous PR, including all, the PR/commit description and code related notes, kudos on that, I much appreciate the effort you put into this.

@ParaskP7
ParaskP7 merged commit 4910b60 into wordpress-mobile:trunk Oct 20, 2022
@Hoossayn

Copy link
Copy Markdown
Contributor Author

👋 @Hoossayn !

Thank you so much for re-opening this PR! 🎉

I reviewed and tested this PR as per the description, along with triggering CI on it via a draft #17361 PR, which I just pushed on the main repo.

Everything LGTM, you did a great job in applying all my suggestion from the previous PR, including all, the PR/commit description and code related notes, kudos on that, I much appreciate the effort you put into this.

👋🏾 @ParaskP7 thank you so much, will open another PR for mediaPickerFragment/viewmodel soon. will do well to adhere to all the suggestions from previous PR

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