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

Relocate all Bridgetown Roda logic to new bridgetown_server plugin #737

Merged
merged 4 commits into from
Apr 10, 2023

Conversation

jaredcwhite
Copy link
Member

@jaredcwhite jaredcwhite commented Apr 1, 2023

This is an alternate take on the concepts brought forth by @michaelherold in #706

Rather than have RodaApp subclass from a Bridgetown-specific subclass of Roda, you can simply subclass from Roda like any standard Roda application, and then use the bridgetown_stack* plugin to pull in all of Bridgetown's setup.

This provides the added benefits of future improvements where some of the ways in which the stack plugin sets up the app could be further configurable as plugin options.

* Still thinking about the best name to call this. Also there's opportunity to consolidate some of the existing plugin(s) into this new one.

** Thinking of going with bridgetown_init unless there are any objections.

*** Core team consensus was to go with bridgetown_server – thanks @KonnorRogers!

- Also catch syntax errors in dev watchers
@render
Copy link

render bot commented Apr 2, 2023

@render
Copy link

render bot commented Apr 2, 2023

@jaredcwhite
Copy link
Member Author

@michaelherold I'm curious what your thoughts are on this PR!

@jaredcwhite jaredcwhite changed the title Relocate all Bridgetown Roda logic to new bridgetown_stack plugin Relocate all Bridgetown Roda logic to new bridgetown_init plugin Apr 6, 2023
@ayushn21
Copy link
Member

ayushn21 commented Apr 6, 2023

Can we just call the plugin bridgetown? I think a Roda plugin called bridgetown makes the most sense if we can do that ...

@jaredcwhite
Copy link
Member Author

@ayushn21 I believe I tried that and ran into some annoying namespace issues, but I'll see if I can come up with a workaround. I agree that would probably be ideal.

@jaredcwhite jaredcwhite changed the title Relocate all Bridgetown Roda logic to new bridgetown_init plugin Relocate all Bridgetown Roda logic to new bridgetown_server plugin Apr 7, 2023
@brandondrew
Copy link

bridgetown_server seems like the clearest possible name 👍

@jaredcwhite
Copy link
Member Author

LGTM!

@jaredcwhite jaredcwhite merged commit aa4f004 into main Apr 10, 2023
@jaredcwhite jaredcwhite deleted the relocate-roda-logic-to-plugin branch April 10, 2023 23:15
@jaredcwhite jaredcwhite added this to the 1.3 milestone Apr 10, 2023
@michaelherold
Copy link
Contributor

I think that this will make testing easier without using integration tests, but haven't had the creative energy available to test it. Good idea, Jared!

Once I am able, I will post an experience report on the new structure.

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.

4 participants