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

Update some libraries #7263

Merged
merged 2 commits into from
Oct 22, 2021
Merged

Update some libraries #7263

merged 2 commits into from
Oct 22, 2021

Conversation

TacoTheDank
Copy link
Member

@TacoTheDank TacoTheDank commented Oct 16, 2021

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

Anything worth implementing from LeakCanary? There's a few new detections for leaks that've been added.

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@TacoTheDank
Copy link
Member Author

TacoTheDank commented Oct 16, 2021

Also, why is BaseFragment watched for leaks? I need to have a specific statement to be able to fix the deprecation.

image

Here's the context:

@Deprecated(
      "Add description parameter explaining why an object is watched to help understand leak traces.",
      replaceWith = ReplaceWith(
          "watch(watchedObject, \"Explain why this object should be garbage collected soon\")"
      )
  )
  @Synchronized fun watch(watchedObject: Any) {
    watch(watchedObject, "")
  }

@litetex litetex marked this pull request as draft October 16, 2021 22:59
@litetex
Copy link
Member

litetex commented Oct 16, 2021

@TacoTheDank

I converted the PR to a draft as there are build problems.

PS: Regarding #7263 (comment)

Also, why is BaseFragment watched for leaks? I need to have a specific statement to be able to fix the deprecation.

No idea but it was added with 0cae58c#diff-b4f682e5acf9f776d048319b275755bb062663781ae2d667f33973bf5eab793aR88

@TacoTheDank TacoTheDank marked this pull request as ready for review October 19, 2021 21:36
@TacoTheDank
Copy link
Member Author

LeakCanary can be updated separately. I don't really feel like letting it hold up the rest of the PR lol.

Copy link
Member

@litetex litetex left a comment

Choose a reason for hiding this comment

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

LGTM

Tested it roughly, no problems

@litetex litetex merged commit 94dfabf into TeamNewPipe:dev Oct 22, 2021
@TacoTheDank TacoTheDank deleted the moreBumps branch October 22, 2021 17:22
This was referenced Nov 30, 2021
@TacoTheDank TacoTheDank added the codequality Improvements to the codebase to improve the code quality label Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
codequality Improvements to the codebase to improve the code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants