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

Make getFormulaFragmentId() a public function. #400

Merged
merged 1 commit into from
Oct 8, 2024

Conversation

Laimiux
Copy link
Collaborator

@Laimiux Laimiux commented Oct 8, 2024

What

Making this into a public API.

@Laimiux Laimiux requested a review from Jawnnypoo October 8, 2024 17:55
@carrotkite
Copy link

JaCoCo Code Coverage 97.16% ✅

Class Covered Meta Status
com/instacart/formula/android/FragmentId 100% 0%
com/instacart/formula/android/FormulaFragment 88% 0%
com/instacart/formula/android/internal/FragmentFlowRenderView 72% 0%
com/instacart/formula/android/internal/FragmentLifecycle 75% 0%

Generated by 🚫 Danger

Copy link
Member

@Jawnnypoo Jawnnypoo left a comment

Choose a reason for hiding this comment

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

Is it expensive for it to generate a new one each time?

@Laimiux
Copy link
Collaborator Author

Laimiux commented Oct 8, 2024

Is it expensive for it to generate a new one each time?

What do you mean?

@Laimiux Laimiux merged commit 2f1ff1d into master Oct 8, 2024
4 checks passed
@Laimiux Laimiux deleted the laimonas/get-formula-fragment-id branch October 8, 2024 18:11
key = getFragmentKey()
)
}
Copy link
Member

Choose a reason for hiding this comment

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

Every time someone calls this, it generates a new FragmentId object, which may be unexpected. Usually a get function doesn't allocate anything new, so this might be unexpected. But I don't know the usecase of this, so it could be fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a tiny data class, should be completely fine.

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.

3 participants