-
Notifications
You must be signed in to change notification settings - Fork 0
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 Portal API #22
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
khamusa
force-pushed
the
feature/2-portal-api-v2
branch
from
January 10, 2025 20:10
6b8b4be
to
35a31eb
Compare
khamusa
force-pushed
the
feature/2-portal-api-v2
branch
2 times, most recently
from
January 10, 2025 20:34
4d78e94
to
3d80462
Compare
Given we don't want the original concrete class to be redefined, we subclass it before evaluating the inline block on its subclass. However, getting this to work was trickier than I anticipated, because the class instance variables we were using on the section DSL configuration would not be inherited by subclasses. After experimenting with a variety of approaches, I decided to pull active support's class_attribute extension. It's a shame that this brings in a bunch of other dependencies, but for now this seemed like the best solution. I also had to deal with the confusion around test_ids when using a concrete class. I decided the concrete class test id will never be used, unless when the class is being explicitly .loaded. (ref: #17). This feels more consistent, and foster code reuse.
khamusa
force-pushed
the
feature/19-inline-extension-of-concrete-classes
branch
from
January 14, 2025 19:48
bd6a232
to
4850d43
Compare
khamusa
force-pushed
the
feature/2-portal-api-v2
branch
from
January 14, 2025 19:48
3d80462
to
c34cda3
Compare
silvabox
reviewed
Jan 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@khamusa I'm just making some arbitrary comments as I parse through this - I'm still getting to grips with the structure and intent of it, so please take everything with a pinch of salt 😅
Co-authored-by: Ben Forrest <[email protected]>
- To foster code reuse, portals can have a global concrete class configured. - They can also have a concrete class specified when they're declared within a section. - It is also possible to combine both approaches, but the DSL-declared klass is required to be a subclass of the globally configured one.
khamusa
force-pushed
the
feature/2-portal-api-v2
branch
from
January 21, 2025 20:57
0ef0a71
to
69565c8
Compare
silvabox
approved these changes
Jan 22, 2025
khamusa
changed the base branch from
feature/19-inline-extension-of-concrete-classes
to
main
January 22, 2025 19:12
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implement Portals API. This is a new implementation, that replaces my first attempt at it, because it was way too confusing. I basically realized Section and Portals weren't different at all, and the creation of further subclasses was completely unnecessary.
This implementation is being used in this PR in the anywhere repo (Ci fails because it has not configured to download the gem from the private repo, though - does not matter as we're not merging that one right now)
I also realized that implementing the inline blocks extension feature would prepare the terrain for the portals api, so this PR is a child of this one.
Delivers #2
Check the changes to the README.md file for a full description of the API.