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

Remove unnecessary dep on prometheus logging #311

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

kschoche
Copy link
Contributor

Changes proposed in this PR:

  • i noticed an unnecessary dependency on prometheus logging in lifecycle sidecar so removed it
  • updated go.mod as well and did a tidy
    This has an added effect of reducing the size of the consul-k8s binary by 1MB :-)

How I've tested this PR:
make clean && make

How I expect reviewers to test this PR:
That it compiles.

Checklist:

  • Tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)
    ^ Both unnecessary, just a cleanup

@kschoche kschoche added type/bug Something isn't working type/enhancement New feature or request labels Aug 19, 2020
@kschoche kschoche requested review from a team, lkysow and thisisnotashwin and removed request for a team August 19, 2020 16:15
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

Good catch!

@@ -112,7 +111,7 @@ func (c *Command) Run(args []string) int {
case <-time.After(c.flagSyncPeriod):
continue
case <-c.sigCh:
log.Info("SIGINT received, shutting down")
logger.Info("SIGINT received, shutting down")
Copy link
Member

Choose a reason for hiding this comment

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

omg good find!

@kschoche kschoche merged commit 6fc94bb into master Aug 19, 2020
@kschoche kschoche deleted the remove_unnecessary_prometheus_dep branch August 19, 2020 16:33
thisisnotashwin pushed a commit that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something isn't working type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants