-
Notifications
You must be signed in to change notification settings - Fork 0
Sweep: Create a new "nomad-external-dns" app (β Sandbox Passed) #4
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
Conversation
Rollback Files For Sweep
This is an automated message generated by Sweep AI. |
Apply Sweep Rules to your PR?
This is an automated message generated by Sweep AI. |
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.
Sweep: This block should reside inside of nomad-external-dns/main.go
, not inside of nomad-deployer
. Keep in mind that nomad-external-dns
is its own app with its own initializer, main
function, etc. While each app can pull in public API functions from other apps, the startup of an app should ideally be self-contained, where possible.
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.
sweep: A dedicated LoadConfig
function is not necessary. Ensure to embed config.CommonConfig
(from pkg/config/types.go
) into the app's Config
struct and use config.Load()
(from pkg/config/config.go
) to load the configuration from the proper source.
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.
sweep: Changes to pkg/system/routes.go
are not necessary. The router is created and handled inside of nomad-external-dns/main.go
, and is only passed to pkg/system/routes.go
to register the canned "system" routes onto the app's router.
PR Feedback: π
Description
This pull request introduces a new "nomad-external-dns" app and makes several changes to the codebase.
Summary
Please review and merge this pull request.
Fixes #3.
π Latest improvements to Sweep:
π‘ To get Sweep to edit this pull request, you can: