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

Fix compilation issues and add Makefile #25

Merged
merged 6 commits into from
Feb 22, 2023

Conversation

JAORMX
Copy link
Contributor

@JAORMX JAORMX commented Feb 20, 2023

For loggingx, the function signature changed. This reflects that change and fixes the
call.

This also fixes the wrong reference to the application's configuration.

For devs, a Makefile has been added.

Finally, a CI has also been added to ensure the project builds with newer patches coming. further tests in CI will be added in the future.

The function signature changed. This reflects that change and fixes the
call.

Signed-off-by: Juan Antonio Osorio <[email protected]>
This fixes the compilation issue of wrong reference for the
configuration variable.

also, in order to avoid a package-wide singleton for the configuration,
this instead reads the configuration into a variable which is then
propagated into the given sub-commands. This makes the sub-command
functions easier to test in the future.

Signed-off-by: Juan Antonio Osorio <[email protected]>
@JAORMX JAORMX changed the title Fix logginx parameter registration Fix compilation issues and add Makefile Feb 22, 2023
Signed-off-by: Juan Antonio Osorio <[email protected]>
fishnix
fishnix previously approved these changes Feb 22, 2023
.github/workflows/test.yaml Outdated Show resolved Hide resolved
.github/workflows/test.yaml Outdated Show resolved Hide resolved
@@ -49,18 +49,18 @@ func init() {
otelx.MustViperFlags(viper.GetViper(), serverCmd.Flags())
}

func serve(ctx context.Context) {
err := otelx.InitTracer(config.AppConfig.Tracing, appName, logger)
func serve(ctx context.Context, cfg *config.AppConfig) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 💯

JAORMX and others added 2 commits February 22, 2023 15:44
Co-authored-by: E Camden Fisher <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
Co-authored-by: E Camden Fisher <[email protected]>
Signed-off-by: Juan Antonio Osorio <[email protected]>
@JAORMX JAORMX requested a review from fishnix February 22, 2023 13:45
&& go install github.com/daixiang0/gci@latest

.PHONY: nk-tool
nk-tool:
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless we're needing NATS related bits in this repo. Probably don't need this.

@JAORMX JAORMX merged commit fb3263c into infratographer:main Feb 22, 2023
@JAORMX JAORMX deleted the fix-loggingx branch February 22, 2023 15:10
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.

3 participants