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

Start app as application #59

Merged
merged 24 commits into from
Feb 3, 2025
Merged

Start app as application #59

merged 24 commits into from
Feb 3, 2025

Conversation

GuzekAlan
Copy link
Contributor

No description provided.

@@ -0,0 +1,75 @@
defmodule LiveDebugger.Helpers do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if this is a correct name for this module but the LiveDebugger will have application setup so button needs to me moved elsewhere.

Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding our IRL conversation using static assets works fine. These were the LiveBook errors which I've seen

README.md Outdated
Comment on lines 20 to 24
config :live_debugger, LiveDebugger.Endpoint,
http: [port: 4007], # Add port on which you want debugger to run
secret_key_base: <SECRET_KEY_BASE>, # Generate secret using `mix phx.gen.secret`
live_view: [signing_salt: <SIGNING_SALT>], # Random 12 letter salt
adapter: Bandit.PhoenixAdapter # Change to your adapter if other is used (see your Endpoint config)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel like we should've add some default values and allow user to modify them but adding some default secrets seems a little bit weird.

README.md Outdated
Comment on lines 29 to 35
And add the debug button to your live layout:
For easy navigation add the debug button to your live layout

```Elixir
# lib/my_app_web/components/app.html.heex

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I won't find solution to the waring in prod in this task so I've left this comment (not everyone needs to use button)

@GuzekAlan GuzekAlan force-pushed the start-app-as-application branch from 0ec4be2 to 3486226 Compare January 30, 2025 19:02
@GuzekAlan GuzekAlan requested a review from kraleppa January 30, 2025 19:06
@GuzekAlan
Copy link
Contributor Author

I've checked it with LiveBook and newly created LiveView app on prod and dev env and it works fine.

@@ -36,6 +36,11 @@ defmodule LiveDebuggerDev.Runner do
]

{:ok, _} = Supervisor.start_link(children, strategy: :one_for_one)

# For some reason `Application.put_env` doesn't work and LiveDebugger starts without config
Application.stop(:live_debugger)
Copy link
Member

Choose a reason for hiding this comment

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

You've already started LiveDebugger via supervisor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've struggled a lot with it and for some reason LiveDebugger.Endpoint has proper configuration but the application doesn't start. I assume that the code of our application might be run before this line but I am not 100% sure

@@ -36,6 +36,11 @@ defmodule LiveDebuggerDev.Runner do
]

{:ok, _} = Supervisor.start_link(children, strategy: :one_for_one)

# For some reason `Application.put_env` doesn't work and LiveDebugger starts without config
Copy link
Member

Choose a reason for hiding this comment

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

We'll have to discuss it on Monday

@@ -0,0 +1,75 @@
defmodule LiveDebugger.Helpers do
Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine

@GuzekAlan GuzekAlan merged commit 799f57a into main Feb 3, 2025
1 check passed
@GuzekAlan GuzekAlan deleted the start-app-as-application branch February 4, 2025 09:17
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.

2 participants