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

Can you suggest any good way to post Compose preview differences in the PR? #354

Closed
takahirom opened this issue Sep 10, 2022 · 9 comments · Fixed by #616
Closed

Can you suggest any good way to post Compose preview differences in the PR? #354

takahirom opened this issue Sep 10, 2022 · 9 comments · Fixed by #616
Assignees

Comments

@takahirom
Copy link
Member

Idea Description
I would like to see the diff of Compose Preview in the Pull Request comment.
Now, we can store the difference in GitHub artifact and comment the workflow URL🎉
But if we use GitHub artifact, the artifact will make a zip file and we can't access in the image file from PR comment.
Is there any idea to show the diff?

Reference images and links
#97

@takahirom takahirom changed the title Can you suggest any good way to post the preview difference in the PR? Can you suggest any good way to post Compose preview differences in the PR? Sep 10, 2022
@eneim
Copy link
Contributor

eneim commented Sep 11, 2022

Did a research and so far this is what I can achieve at the moment. Still too early and dirty but please take a look.

eneim#1 (comment) (Too lazy to fake a failed test, so I just pick the first screenshot and put it to the comment. Depending on what you want to do, we can include all failed screenshots in the comment, but I have not tried with multiple images yet.)

The key step: https://github.com/eneim/conference-app-2022/blob/ec4c83e9f836ac81559708f7eacf5c6d4d9b6ca4/.github/workflows/ScreenShotTest.yml#L51

The point is to use iterative/setup-cml to build a markdown with images from local files.

@takahirom
Copy link
Member Author

takahirom commented Sep 11, 2022

Probably asset.cml.dev/ will create the githubusercontent url, right?
The githubusercontent is public, but we all use it. So I think it is OK. But I don't know much about "asset.cml.dev".
But they have a privacy policy. So probably OK.
https://iterative.ai/security-and-privacy

@eneim
Copy link
Contributor

eneim commented Sep 11, 2022

Thanks, good found. I just found out about their custom image host too. Given the fact that there is no non-UI method to upload images to github's comment, I think automating this step will require the images to be hosted somewhere.

On one hand, it seems to be safe to borrow assets.cml.dev resource due to their policy. But on the other hand, the screenshot may include data shown in the application. Please help to double-check with some team members to confirm if we can use this approach or not :D.

There is some other options:

  • Self-host the screenshot hosting (cml supports it via `cml publish --url=""). Some work to do and we need a compatible cloud storage, not free as we want it to be :D.
  • Not using cml publish to upload the screenshot, but some other cli-base image hostings (such as Cloudiary). Can increase the complexity of the work.

We always have the option to ... not doing this because we can download the artifact and verify the issue manually.

What do you think? If we can keep using cml as-is, I can continue with the PR :D. Otherwise, we can re-visit this ticket another day.

@eneim
Copy link
Contributor

eneim commented Sep 13, 2022

Ping @takahirom so it is Ok to use cml as-is?

@takahirom
Copy link
Member Author

takahirom commented Sep 13, 2022

Sorry for keeping you waiting.
I would like to introduce it in production application ideally. cml way can be controversial.

How about creating a branch for the image and remove when PR close?
https://github.com/DroidKaigi/conference-app-2022/raw/main/.idea/icon.png
test

@eneim
Copy link
Contributor

eneim commented Sep 15, 2022

@takahirom Got it. I can give it another try this weekend. At the mean time, if there is any promising approach, feel free to move ahead and let me know.

@eneim
Copy link
Contributor

eneim commented Sep 19, 2022

@takahirom eneim#1 (comment) FYI, it can be overwhelming if the number of images are big. But I kinda make it works without cml. Will prepare a PR with more details.

@takahirom
Copy link
Member Author

@eneim Great! I think we can filter later if there are the same composable function preview images. I'm really looking forward to seeing it!

@eneim
Copy link
Contributor

eneim commented Sep 19, 2022

@takahirom #616

The current idea is quite similar to your comment above, except that it uses the repository's github page (https://droidkaigi.github.io/conference-app-2022/, when enabled). It seems that images served by that domain will be easier to work with.

I may need your help to configure the DroidKaigi/conference-app-2022 repository to make it work. The PR has the "required action items" so please help to take a look and discuss next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants