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.

Sign up for GitHub

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

Jump to bottom

Add README section about Compose preview comment #647

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Add README section about Compose preview comment #647

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8563264
Add README section about Compose preview comment
takahirom Sep 21, 2022
13069e5
Fix small typo
takahirom Sep 21, 2022
a2d70e6
Apply review
takahirom Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,23 @@ We are trying to draw a timetable using LazyLayout, a base implementation of Laz

https://github.com/DroidKaigi/conference-app-2022/blob/91715b461b3162eb04ac58b79ba39ccdf21cf222/feature-sessions/src/main/java/io/github/droidkaigi/confsched2022/feature/sessions/Timetable.kt#L73

## Jetpack Compose preview in GitHub comments

If we can check Compose's Preview during GitHub code review, we can efficiently review changes.
takahirom marked this conversation as resolved.
Show resolved Hide resolved
For the DroidKaigi app, we have addressed several issues in addition to the existing Paparazzi + Showkase approach.

* Committing Preview images to the GitHub repository increases the GitHub repository size.
* When uploading preview images to the cloud, the images may not be viewable depending on their permissions.

In the DroidKaigi app, you can use GitHub Actions to save a golden snapshot of the main branch using GitHub Actions Artifacts, create a companion branch for the pull request, upload the preview image there, and commit the image to GitHub. Comment the URL of the image.
Comment on lines +281 to +286
Copy link
Contributor

@eneim eneim Sep 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If I understand correctly, you want to mention what we have tried to adding the images to the PR comments? If so, how about the following change:

There are different options we have considered to add the delta images to the pull-request comments, but each of them has some challenges:

  • Committing images to the main branch: this option increases the GitHub repository size unnecessarily. Once we resolve the preview issue, the delta images are no longer needed. // Review note: my understanding is that you want to explain why we don't want to store the delta screenshots in the repository permanently. So it should be about the main branch. But this is just my guess, please feel free to express your intent about this part.
  • Uploading images to the cloud: the snapshot may contains sensitive information that we should not upload them elsewhere. And also, the cost to maintain an additional image storage is not trivial. // Reviewer note: I think the images may not be viewable depending on their permissions is not a strong reason, because most of the cloud allows granular access control. But again, this change is just my guess from your texts, so feel free to change it your way.

In the DroidKaigi app, we use GitHub Actions to handle this effectively:

  • The golden snapshots are stored on the main branch using GitHub Actions Artifacts.
  • For the pull-request, we use GitHub Actions to create a companion branch and upload the delta images there. The image URLs are used to comment on the pull-request's branch.
  • Once the preview issues are resolved, we can delete the companion branch. (Currently this step is not automated).

Just a comment from my understanding, it may not reflect what you have in mind correctly, so feel free to discard 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.

Let me take a look at this 👀


<img src="https://user-images.githubusercontent.com/1386930/191486713-178afc67-91a7-47e3-9213-81f1326367e6.png" width="650px" />

This function was developed mainly by these two people.

* [@eneim](https://github.com/eneim)
* [@sobaya-0141](https://github.com/sobaya-0141)

## Special Thanks
* [Contributors](https://github.com/DroidKaigi/conference-app-2022/graphs/contributors)
* iOS App Leader [ry-itto / Ryoya Ito](https://github.com/ry-itto)
Expand Down