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

Restrict Paparazzi's public API #1264

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Conversation

jrodbx
Copy link
Collaborator

@jrodbx jrodbx commented Jan 26, 2024

As a precursor to the SDKifying work, I've sadly realized that we haven't done a great job of restricting the API surface of the project. This PR attempts to correct that by adding the kotlin binary validation plugin + slapping internal on a lot of things.

One thing to call out is this PR will make HtmlReportWriter and SnapshotVerifier internal as well. I felt keeping SnapshotHandler public was enough, but due to some current design shortcomings in the snapshot pipeline (to be addressed at some point!), that level of strictness might break current consumers of the project (which I'd like to avoid!) who have implemented workarounds.

Therefore, I'd be open to reverting those 2 classes (and others!) for the time being, until we come up with a better solution/design. Please speak up!

cc: @TWiStErRob

@jrodbx
Copy link
Collaborator Author

jrodbx commented Jan 26, 2024

that level of strictness might break current consumers of the project (which I'd like to avoid!) who have implemented workarounds.

@gamepro65 kindly reminds me that we have also internally implemented a workaround using HtmlReportWriter and SnapshotVerifier LOL. ok, reverting those for now...

@jrodbx jrodbx force-pushed the jrod/2024-01-26/api-tightening branch from 5bcff1f to a20c95a Compare January 27, 2024 08:53
build.gradle Outdated Show resolved Hide resolved
paparazzi/api/paparazzi.api Outdated Show resolved Hide resolved
paparazzi/api/paparazzi.api Show resolved Hide resolved
paparazzi/api/paparazzi.api Show resolved Hide resolved
paparazzi/api/paparazzi.api Show resolved Hide resolved
paparazzi/api/paparazzi.api Show resolved Hide resolved
@jrodbx jrodbx force-pushed the jrod/2024-01-26/api-tightening branch 2 times, most recently from 39b7705 to 44a138a Compare January 29, 2024 05:48
@jrodbx jrodbx force-pushed the jrod/2024-01-26/api-tightening branch 3 times, most recently from 2dabf52 to 1bca5c9 Compare January 29, 2024 21:23
@jrodbx jrodbx force-pushed the jrod/2024-01-26/api-tightening branch from b2cb074 to 95baa53 Compare January 30, 2024 02:38
@jrodbx jrodbx merged commit 3a9c371 into master Jan 30, 2024
14 checks passed
@jrodbx jrodbx deleted the jrod/2024-01-26/api-tightening branch January 30, 2024 03:17
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.

6 participants