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

Always creates SymbolToField "id" instance #1497

Merged

Conversation

hw202207
Copy link
Contributor

@hw202207 hw202207 commented May 18, 2023

Context

  • Regardless the type of primary key, there is always a {EntityName}Id type hence I think it's safe to always create a SymbolToField instance for ID.

@hw202207 hw202207 force-pushed the haisheng/always-create-id-symbol-field branch from c02adcb to beb58c2 Compare May 18, 2023 06:06
@hw202207 hw202207 force-pushed the haisheng/always-create-id-symbol-field branch from beb58c2 to 17cbadb Compare June 28, 2023 17:44
@hw202207 hw202207 marked this pull request as ready for review June 28, 2023 17:46
- given there is always a {EntityName}Id type
@hw202207 hw202207 force-pushed the haisheng/always-create-id-symbol-field branch from 4b90e8d to 04728c2 Compare June 28, 2023 19:03
@hw202207 hw202207 changed the title Always create SymbolToField for "id" Always creates SymbolToField "id" instance Jun 28, 2023
Comment on lines 3 to 6
## Unreleased

* [1497](https://github.com/yesodweb/persistent/pull/1497)
* Always generates `SymbolToField "id"` instance
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will need to be a minor version bump - may be best to roll up with #1503

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable to me. Anything I need to do / pay attention to?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I've finished everything that needed to be done in #1503 (re-requested review to be sure). Aside from this PR, is anything else planned for 2.14.6.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll rebase after #1503 has been merged (adjust the changelog section)

@parsonsmatt parsonsmatt added this to the 2.14.6 milestone Sep 18, 2023
@parsonsmatt
Copy link
Collaborator

This builds and tests just fine locally so I am going to merge. Thank you!

@parsonsmatt parsonsmatt merged commit c88f5ce into yesodweb:master Oct 3, 2023
2 of 7 checks passed
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.

None yet

3 participants