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

684-restore-localhost-url-with-initializers #686

Merged
merged 2 commits into from
Jan 2, 2023

Conversation

jaredcwhite
Copy link
Member

Fixes issue #684

Also removes the missing configuration file warning if an initializers-based config is available

@render
Copy link

render bot commented Dec 28, 2022

@render
Copy link

render bot commented Dec 28, 2022

@@ -144,9 +144,11 @@ def run_initializers!(context:)
Bridgetown.logger.debug "Initializing:", "Running initializers with `#{context}' context in:"
Bridgetown.logger.debug "", initializers_file
self.init_params = {}
cached_url = url&.include?("//localhost") ? url : nil
Copy link
Contributor

Choose a reason for hiding this comment

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

question: How else other than running bin/bt dev can the value be set at this point? I'm unclear why we need to grab this cached_url because of my lack of understanding here.

Do we need to grab it because the port is overrideable? Because it can be set in other ways?

My hypothesis is that perhaps a bridgetown.config.yml has already been processed and we need to account for a value in there. Am I on the right track?

Copy link
Member Author

Choose a reason for hiding this comment

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

@michaelherold it's overridden by the start command when in development environment before any other configuration has been loaded, even if there's a url key in the YAML config. But then when the initializers code is executed, setting url was overriding the localhost overwrite 😅. We want to undo that if necessary. There may be a more elegant way to refactor this in the near future…

@jaredcwhite jaredcwhite merged commit fdf92d6 into main Jan 2, 2023
@jaredcwhite jaredcwhite deleted the 684-restore-localhost-url-with-initializers branch January 2, 2023 16:52
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