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

Server CLIRunner refactor #2436

Merged
merged 2 commits into from Oct 5, 2022
Merged

Server CLIRunner refactor #2436

merged 2 commits into from Oct 5, 2022

Conversation

hmdne
Copy link
Member

@hmdne hmdne commented Sep 17, 2022

Refactor the Server CLI Runner to use Opal::SimpleServer

  • We extend the API here - if runner.dynamic_builder? then it doesn't
    provide the runners with a Builder, but a proc that returns a Builder
    which will always correspond to an up-to-date code.
  • As a result - now the server runner will supply an up-to-date code
    each refresh.

@hmdne hmdne added this to the v1.6 milestone Sep 18, 2022
Copy link
Member

@elia elia left a comment

Choose a reason for hiding this comment

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

Awesome, thoughts on going a step further and always using a dynamic-builder for all runners?

My guess is that the real difference is that some runners will only fetch the builder once, while others will do it multiple times. That way we can avoid some complexity.

This is so that at this time we have a DOM tree ready, so we don't
have to explicitly do on(:dom_ready) in order to receive $document.body
Copy link
Member

@elia elia left a comment

Choose a reason for hiding this comment

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

Live reviewed together and paired on the previously requested changes

* We change the API here, now the provided builder is a proc that
  returns a new Builder instance which will always correspond to an
  up-to-date code.
* As a result - now the server runner will supply an up-to-date code
  each refresh.
* Also refactors the Server CLI Runner to use Opal::SimpleServer.
* Ensure files are rewinded as they need to be re-read at each refresh
@elia elia merged commit 20b0e9f into master Oct 5, 2022
@elia elia deleted the hmdne/server-runner branch October 5, 2022 08:46
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

2 participants