-
Notifications
You must be signed in to change notification settings - Fork 109
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
Implement association as it's own thing #407
Comments
Happy to discuss this in more detail, and elaborate more. |
Friendly ping @lessthanjacob @njbbaer @ritikesh If you think this is a reasonable suggestion, let me know! If so, I'd propose these steps:
For reference, there are also a few other issues that I've opened, where I'd also be happy to get your input. Full list |
Friendly ping @lessthanjacob @njbbaer @ritikesh Happy to hear your thoughts on this! |
What would the responsibilities be of this Association class outside of how it's being handled within the ViewCollection/View contexts today? |
It would be a sibling class next to Mostly just seemed like a design that's easier to comprehend, because right now an association is a field with the "magic" option called That said, I don't feel strongly about this. An alternative would be to turn it into a property on the field (similar to what's proposed for the other magic options in #405). |
They do behave similarly. It's just a readability thing with no major functional differences barring associations having their own blueprinter/view based rendering.
At this point, I'm not strongly for/against this one either. But the trend seems to be in the direction of major rewrites and as long as there's no/little impact to the consumers, this should still be ok. |
in fact, many fields have additional options too - like date_time formatter (which we're planning to get rid off too in favour of extractors). If we're able to achieve consistency in terms of the options that required for defining a field and an association with them being unique to their own types, then I think this might make more sense. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Not stale |
I didn't link this issue, but I recently started down this path with #414, albeit in a very rudimentary way for the time being. blueprinter/lib/blueprinter/association.rb Lines 1 to 33 in 2150b92
This makes the interface a bit more explicit/clearer, but there's room to take this further, since it's mostly functioning as a facade around |
That's great! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Not stale |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Not stale |
Is there an existing issue for this?
Is your feature request related to a problem? Please describe
Right now, association is internally implemented as a field, with a magic
:assocation
option.Describe the feature you'd like to see implemented
I'd implement association as a proper class internally, similar to
Field
.Describe alternatives you've considered
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: