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

Combine the two Constants files into one file. #5418

Merged
merged 1 commit into from
Jan 15, 2021

Conversation

Isira-Seneviratne
Copy link
Member

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

  • Combine Constants.java and ConstantsKt.kt into one file.

APK testing

debug.zip

Due diligence

/**
* Default duration when using throttle functions across the app, in milliseconds.
*/
const val DEFAULT_THROTTLE_TIMEOUT = 120L
Copy link
Member

Choose a reason for hiding this comment

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

I see there are no other file changes. Isn't this variable used anywhere? If that's the case remove it

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used in some of the ViewModels.

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the ViewModels imports also be changed? Otherwise they would point to ConstantsKt, wouldn't they?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ViewModels are written in Kotlin, so no change is needed.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Ok, thank you

@Stypox Stypox merged commit c216f29 into TeamNewPipe:dev Jan 15, 2021
@Isira-Seneviratne Isira-Seneviratne deleted the Unify_constants branch January 15, 2021 09:28
This was referenced Jan 18, 2021
tossj pushed a commit to tossj/NewPipe-legacy that referenced this pull request Apr 20, 2021
…ify_constants

Combine the two Constants files into one file.
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.

2 participants