Skip to content

New Image component (2023-04) #787

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
Merged
lordofthecactus merged 13 commits into from
Apr 18, 2023
Merged

New Image component (2023-04) #787

lordofthecactus merged 13 commits into from
Apr 18, 2023

Conversation

@benjaminsehl
Copy link
Member

@benjaminsehl benjaminsehl commented Apr 13, 2023

Replaces #649

@github-actions

This comment has been minimized.

Sign in to view
@lordofthecactus lordofthecactus force-pushed the Image-23-04 branch 2 times, most recently from 4853bd0 to d357a01 Compare April 17, 2023 15:18
@lordofthecactus lordofthecactus changed the base branch from 2023-04-POC to 2023-04 April 17, 2023 15:18
cartogram
cartogram reviewed Apr 17, 2023
View reviewed changes
Copy link
Contributor

@cartogram cartogram left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Left some comments on the changeset cause I hadn't read that yet, but I am not sure how this is different from #649 so not sure what needs to be re-reviewed 😕 Why are all the generated files included here?

benjaminsehl and others added 8 commits April 18, 2023 10:39
@benjaminsehl @lordofthecactus
Creates new Image on top of 2023-04
71d8339 
Co-authored-by: Matt Seccafien <matt.seccafien@shopify.com>
@benjaminsehl @lordofthecactus
Adds changeset
3b4dedb
@benjaminsehl @lordofthecactus
Fix tests
71d1582
@benjaminsehl @lordofthecactus
Builds docs
268fd92
@lordofthecactus
update changesets to major
e673e27
@lordofthecactus
Update .changeset/thirty-rice-sin.md
bede8e8 
Co-authored-by: Matt Seccafien <matt.seccafien@shopify.com>
@lordofthecactus
fix docs
b1c59de
@lordofthecactus
Apply suggestions from code review
f7de5ce 
Co-authored-by: Matt Seccafien <matt.seccafien@shopify.com>
lordofthecactus
lordofthecactus reviewed Apr 18, 2023
View reviewed changes
@benjaminsehl
Copy link
Member Author

benjaminsehl commented Apr 18, 2023 •
edited
Loading

Left some comments on the changeset cause I hadn't read that yet, but I am not sure how this is different from #649 so not sure what needs to be re-reviewed 😕

There was some issue with #649 that wasn't building properly on 2023-04 — and so @lordofthecactus and I figured the simplest thing in the end, after a bunch of trial and error, was to just copy/paste and do a new PR rebased on top of 2023-04.

developit
developit approved these changes Apr 18, 2023
View reviewed changes
Copy link

@developit developit left a comment

Choose a reason for hiding this comment

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

Reviewed in person, LGTM.

@lordofthecactus lordofthecactus merged commit 361879e into 2023-04 Apr 18, 2023
@lordofthecactus lordofthecactus deleted the Image-23-04 branch April 18, 2023 14:35
@benjaminsehl benjaminsehl mentioned this pull request Apr 18, 2023
Closed
@github-actions github-actions bot mentioned this pull request Apr 18, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@developit developit developit approved these changes

+2 more reviewers

@cartogram cartogram cartogram left review comments

@lordofthecactus lordofthecactus lordofthecactus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@benjaminsehl @developit @cartogram @lordofthecactus