Rename PreviewParameterProvider subclasses to the PreviewParam suffix - #7468
Open
hayaksi1 wants to merge 1 commit into
Open
Rename PreviewParameterProvider subclasses to the PreviewParam suffix#7468hayaksi1 wants to merge 1 commit into
hayaksi1 wants to merge 1 commit into
Conversation
The `Provider` suffix reads as a dependency provider rather than as preview
data, and it collides with the real providers in the codebase — `StringProvider`,
`PickerProvider`, the login feature's `AccountProvider` — so the name says
nothing about which of the two a class is. `PreviewParam` says it.
195 classes are renamed, along with the 185 files named after them, the konsist
rule that enforces the convention, the presentation-class file templates and the
onboarding documentation.
This cannot be split. `Konsist.scopeFromProduction()` scans the whole project, so
flipping the assertion before every class has moved leaves the rule red, and a
rule that accepted either suffix during a transition would be exactly the kind of
compatibility layer this convention change is meant to remove.
Two judgement calls in the rule itself. The `name.endsWith("PreviewProvider")`
exclusion is dropped: its only job was to reject a redundant `Preview` before
`Provider`, and the new suffix already contains it; no class currently ends with
`PreviewProvider`, so removing it changes nothing today. And
`EditableAvatarViewUriProvider` is renamed rather than deleted from the exclusion
list even though no such class exists in this repository, since it presumably
comes from the modules overlaid for the enterprise build.
Nothing keyed off the old suffix: no snapshot filename contains a provider class
name, so no Paparazzi image changes, and the Kover variants match on `*Presenter`,
`*State` and `*ViewKt`, so coverage bounds are unaffected.
github-actions
Bot
added
the
Z-Community-PR
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Content
The
Providersuffix onPreviewParameterProvidersubclasses reads as a dependency provider rather than as preview data, and it collides with the real providers in the codebase —StringProvider,PickerProvider, and the login feature's ownAccountProviderdomain type — so the name does not say which of the two a class is. This renames them all to aPreviewParamsuffix, as agreed on the issue.195 classes move, along with the 185 files named after them, the konsist rule that enforces the convention, the two presentation-class file templates plus their settings, and the onboarding documentation.
It cannot be split.
Konsist.scopeFromProduction()scans the whole project, so flipping the assertion before every class has moved leaves the rule red, and a rule accepting either suffix during a transition would be the kind of compatibility layer this change exists to avoid.Two judgement calls inside the rule are worth calling out. The
name.endsWith("PreviewProvider").not()clause is dropped, because its only job was to reject a redundantPreviewin front ofProviderand the new suffix already containsPreview; no class currently ends withPreviewProvider, so this changes nothing today. AndEditableAvatarViewUriProvideris renamed in the exclusion list rather than removed, even though no such class exists in this repository — it presumably comes from the modules overlaid for the enterprise build, so anyPreviewParameterProviderstill named*Providerover there will need the same rename before the flipped rule passes for that variant.Nothing else keyed off the old suffix. No snapshot filename contains a provider class name, so there is no Paparazzi churn, and the Kover variants match on
*Presenter,*Stateand*ViewKt, so the coverage bounds do not move.Motivation and context
Part of #4657.
Tests
tests/konsist/.../KonsistClassNameTest.ktis the test for this convention: the rule now requires thePreviewParamsuffix, and its "more than 100 classes found" guard still holds. Run with./gradlew :tests:konsist:testDebugUnitTest --rerun— konsist scans sources at execution time, so a cached run proves nothing../gradlew :tests:uitests:compileDebugUnitTestKotlincompiles every module the rename touches (188 of them) and is green, and./gradlew ktlintFormathandled the import re-ordering that the renames forced.git status --porcelain -- tests/uitests/src/test/snapshotsis empty, which is the check that no snapshot changed.No new test: a naming convention has nothing behavioural to assert beyond the konsist rule.
Tested devices
Checklist