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

feat: Add url query params for quick access #248

Merged
merged 5 commits into from
Dec 16, 2024
Merged

feat: Add url query params for quick access #248

merged 5 commits into from
Dec 16, 2024
+48 −1

Conversation

sasamuku
Copy link
Member

@sasamuku sasamuku commented Dec 13, 2024 •
edited
Loading

Summary

Add url query params for quick access.

2024-12-13.18.10.43.mov

Related Issue

Changes

Testing

Other Information

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Dec 13, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: b1521ed

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@sasamuku sasamuku force-pushed the add_url_query_params_for_quick_access branch from bb09a99 to e7cd087 Compare December 13, 2024 09:02
@sasamuku sasamuku temporarily deployed to preview - erd-sample December 13, 2024 09:02 — with GitHub Actions Inactive
@sasamuku sasamuku marked this pull request as ready for review December 13, 2024 09:18
@sasamuku sasamuku requested a review from a team as a code owner December 13, 2024 09:18
@sasamuku sasamuku requested review from hoshinotsuyoshi, FunamaYukina, junkisai and MH4GF and removed request for a team December 13, 2024 09:18
@MH4GF
Copy link
Member

MH4GF commented Dec 13, 2024

@sasamuku Looks good, but I'm concerned about the lack of focus when it opens!

2024-12-13.18.24.46.mov

Please consider this.

@MH4GF
Copy link
Member

MH4GF commented Dec 13, 2024

It was safe to put in the appropriate table name. 👍🏻

https://liam-erd-sample-6swvq5ps4-route-06-core.vercel.app/?active=not-found-table

MH4GF
MH4GF approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@MH4GF MH4GF left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
nice work!

.changeset/swift-worms-retire.md Outdated
Copy link
Member

Choose a reason for hiding this comment

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

I thought one PR and one changeset would be fine, what do you think? cc @hoshinotsuyoshi

Copy link
Member Author

Choose a reason for hiding this comment

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

This is first time to write changeset, and it was quite hard work...

I'm not sure the best practice to write changeset, but one pr one changeset would be good for us and our contributors.

Copy link
Member

Choose a reason for hiding this comment

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

@sasamuku @MH4GF

one PR and one changeset would be fine

That might be the case.

What really matters is that the .changeset/*.md content is included in a single PR, and I don't think it’s particularly important which commit within the PR it gets merged into.

Copy link
Member

Choose a reason for hiding this comment

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

I agree with Hoshino-san, that changeset is unnecessary for every commit because it is better to write for the user rather than for the contributor!

@hoshinotsuyoshi
Copy link
Member

@sasamuku
Apologies for the inconvenience! I previously mentioned that the .changeset directory would be located at ./.changeset, but it has now been moved back to ./frontend/.changeset. For more details, please refer to #257. I’d greatly appreciate it if you could move the files to ./frontend/.changeset. Thank you so much!

@sasamuku sasamuku force-pushed the add_url_query_params_for_quick_access branch from e7cd087 to 937c4f4 Compare December 16, 2024 02:34
@sasamuku sasamuku temporarily deployed to preview - erd-sample December 16, 2024 02:35 — with GitHub Actions Inactive
@sasamuku sasamuku temporarily deployed to preview - erd-sample December 16, 2024 02:36 — with GitHub Actions Inactive
@sasamuku
Copy link
Member Author

I'm concerned about the lack of focus when it opens!

I'll fix this another PR🚀

@sasamuku sasamuku added this pull request to the merge queue Dec 16, 2024
Merged via the queue into main with commit 9ef734a Dec 16, 2024
12 checks passed
@sasamuku sasamuku deleted the add_url_query_params_for_quick_access branch December 16, 2024 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MH4GF MH4GF MH4GF approved these changes

@hoshinotsuyoshi hoshinotsuyoshi Awaiting requested review from hoshinotsuyoshi hoshinotsuyoshi is a code owner automatically assigned from liam-hq/liam-dev

@FunamaYukina FunamaYukina Awaiting requested review from FunamaYukina FunamaYukina is a code owner automatically assigned from liam-hq/liam-dev

@junkisai junkisai Awaiting requested review from junkisai junkisai is a code owner automatically assigned from liam-hq/liam-dev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sasamuku @MH4GF @hoshinotsuyoshi