Skip to content

Remove runtime dep on androidx fragment navigiation from modules that don't strictly need it#961

Merged
breedx-splk merged 2 commits into
open-telemetry:mainfrom
bidetofevil:service-fragment-dep
Apr 29, 2025
Merged

Remove runtime dep on androidx fragment navigiation from modules that don't strictly need it#961
breedx-splk merged 2 commits into
open-telemetry:mainfrom
bidetofevil:service-fragment-dep

Conversation

@bidetofevil
Copy link
Copy Markdown
Contributor

Remove runtime dependency by looking at whether the fragment is a NavHost, which is not part of the navigation-fragment package. We are still depending on AndroidX's Navigation API because that's what the logic checks. Further decoupling will require the implementation to not know about the concept of NavHost, which will be trickier to do and out of scope for what I'm trying to achieve, i.e. remove the dep to the fragments navigation package only.

@bidetofevil bidetofevil requested a review from a team as a code owner April 22, 2025 21:24
@bidetofevil
Copy link
Copy Markdown
Contributor Author

Added additional commit that made ScreenNameExtractor agnostic to the input class. This enabled me to remove the dependency from all the modules that don't actually need it.

@bidetofevil bidetofevil changed the title Remove runtime dep on androidx fragment navigiation Remove runtime dep on androidx fragment navigiation from modules that don't strictly need it Apr 22, 2025
Copy link
Copy Markdown
Contributor

@LikeTheSalad LikeTheSalad left a comment

Choose a reason for hiding this comment

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

Thank you 🙏

fun testActivity() {
val activity = Activity()
val name = ScreenNameExtractor.DEFAULT.extract(activity)
Assertions.assertEquals("Activity", name)
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.

Please use static imports for these, or ever better, migrate to the fluent assertj assertions: assertThat(name).isEqualTo("Activity").

I know you didn't write these and migrating to kotlin was just part of the cleanup, but it would help here. Thanks!

Comment thread services/build.gradle.kts
@breedx-splk breedx-splk merged commit c5f87c4 into open-telemetry:main Apr 29, 2025
6 checks passed
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