Skip to content

Added log package #177

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 9, 2022
Merged

Added log package #177

merged 1 commit into from
Sep 9, 2022

Conversation

yuriolisa
Copy link
Contributor

@yuriolisa yuriolisa commented Aug 26, 2022 •
edited by skattoju
Loading

fixes #76
Signed-off-by: Yuri Sa [email protected]

  • Added option OPCAP_LOG which can be set in one of these values warn, info, error, debug.
  • Added flag --log-level.
  • Added Zap pkg, implementing the Sugared logger.
  • Changed log messages to the respective log level prefix.

@yuriolisa yuriolisa requested a review from acmenezes August 26, 2022 12:45
@exe-prow-github-app exe-prow-github-app bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 26, 2022
@exe-prow-github-app exe-prow-github-app bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 26, 2022
@yuriolisa
Copy link
Contributor Author

Resolves #76

@yuriolisa yuriolisa linked an issue Aug 26, 2022 that may be closed by this pull request
Improve logger for User and Dev cases #76
Closed
@exe-prow-github-app exe-prow-github-app bot added do-not-merge/contains-merge-commits size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 31, 2022
@yuriolisa yuriolisa marked this pull request as ready for review August 31, 2022 16:01
@exe-prow-github-app exe-prow-github-app bot removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Aug 31, 2022
@exe-prow-github-app exe-prow-github-app bot requested a review from rocrisp August 31, 2022 16:01
@yuriolisa yuriolisa linked an issue Aug 31, 2022 that may be closed by this pull request
Implement Debug level logging as a flag #132
Closed
acmenezes
acmenezes requested changes Aug 31, 2022
View reviewed changes
Copy link
Contributor

@acmenezes acmenezes left a comment

Choose a reason for hiding this comment

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

Hi @yuriolisa, thanks for the PR! The work here looks good but can we stick to zap? I would change my vote here if I can see clear advantages on the hclog library and/or the majority of the team thinks we should go differently.

@yuriolisa yuriolisa requested a review from acmenezes September 1, 2022 13:23
yashoza19
yashoza19 reviewed Sep 1, 2022
View reviewed changes
@yuriolisa yuriolisa requested a review from yashoza19 September 1, 2022 14:07
bcrochet
bcrochet requested changes Sep 1, 2022
View reviewed changes
Copy link
Contributor

@bcrochet bcrochet left a comment

Choose a reason for hiding this comment

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

Comments and suggestions.

@opdevbot opdevbot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 1, 2022
@opdevbot opdevbot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 2, 2022
bcrochet
bcrochet requested changes Sep 8, 2022
View reviewed changes
Copy link
Contributor

@bcrochet bcrochet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Seems like the merge commits are still present. This really needs to be collapsed to a single commit. And it looks like a make tidy is necessary.

@yuriolisa yuriolisa requested a review from bcrochet September 9, 2022 08:37
@opdevbot opdevbot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2022
@exe-prow-github-app exe-prow-github-app bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 9, 2022
@opdevbot opdevbot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 9, 2022
@exe-prow-github-app exe-prow-github-app bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 9, 2022
yashoza19
yashoza19 requested changes Sep 9, 2022
View reviewed changes
@yuriolisa
Requested changes to logger package
e002aa6 
Signed-off-by: Yuri Sa <[email protected]>

Fixed some linters issues

Signed-off-by: Yuri Sa <[email protected]>

Removing ci linters

Signed-off-by: Yuri Sa <[email protected]>

Removing periods

Signed-off-by: Yuri Sa <[email protected]>

Removing periods

Signed-off-by: Yuri Sa <[email protected]>

Removing periods

Signed-off-by: Yuri Sa <[email protected]>

Added flag logic

Signed-off-by: Yuri Sa <[email protected]>

Fixed errors checking

Signed-off-by: Yuri Sa <[email protected]>

Fixed linters comments

Signed-off-by: Yuri Sa <[email protected]>

Fixed linters comments

Signed-off-by: Yuri Sa <[email protected]>

Fixed linters comments

Signed-off-by: Yuri Sa <[email protected]>

Rebase

Remove redundant text from check cmd description

The flags and examples are automatically generated. There is no need
to maintain the extra text.

Fixes opdev#201

Signed-off-by: Brad P. Crochet <[email protected]>

Fix gofumpt errors

Once the actions were turned on, gofumpt started complaining. This
should alleviate that.

Signed-off-by: Brad P. Crochet <[email protected]>

add all-installmodes flag to check command

Pass the same context throughout the stack

The context should be created at the highest level, and the same
context should be passed down through the code. Otherwise, it
defeats the purpose of using the context in the first place.

Fixes opdev#207

Signed-off-by: Brad P. Crochet <[email protected]>

Fixed based on requests comments

Signed-off-by: Yuri Sa <[email protected]>

Fixed based on requests comments

Signed-off-by: Yuri Sa <[email protected]>

Fixing go issues

Signed-off-by: Yuri Sa <[email protected]>

Fixing go issues

Signed-off-by: Yuri Sa <[email protected]>

Fixing go issues

Signed-off-by: Yuri Sa <[email protected]>

Fixed linters

Signed-off-by: Yuri Sa <[email protected]>

Fixed rebase

Signed-off-by: Yuri Sa <[email protected]>

Fixed issues

Signed-off-by: Yuri Sa <[email protected]>
yashoza19
yashoza19 approved these changes Sep 9, 2022
View reviewed changes
Copy link
Collaborator

@yashoza19 yashoza19 left a comment

Choose a reason for hiding this comment

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

/approve

@yuriolisa yuriolisa requested a review from yashoza19 September 9, 2022 13:32
@acmenezes
Copy link
Contributor

/lgtm

@exe-prow-github-app exe-prow-github-app bot added the lgtm Indicates that a PR is ready to be merged. label Sep 9, 2022
acmenezes
acmenezes approved these changes Sep 9, 2022
View reviewed changes
Copy link
Contributor

@acmenezes acmenezes left a comment

Choose a reason for hiding this comment

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

Looks great now.

skattoju
skattoju approved these changes Sep 9, 2022
View reviewed changes
Copy link
Contributor

@skattoju skattoju left a comment

Choose a reason for hiding this comment

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

/lgtm

skattoju
skattoju approved these changes Sep 9, 2022
View reviewed changes
Copy link
Contributor

@skattoju skattoju left a comment

Choose a reason for hiding this comment

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

/approve

@acmenezes
Copy link
Contributor

/approve no-issue

@exe-prow-github-app exe-prow-github-app bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 9, 2022
@exe-prow-github-app
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: acmenezes, skattoju, yashoza19

Associated issue: #76

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [acmenezes,yashoza19]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@exe-prow-github-app exe-prow-github-app bot merged commit 83a55db into opdev:main Sep 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@exe-prow-github-app exe-prow-github-app[bot] exe-prow-github-app[bot] left review comments

@acmenezes acmenezes acmenezes approved these changes

@skattoju skattoju skattoju approved these changes

@makon57 makon57 Awaiting requested review from makon57

@spabba17 spabba17 Awaiting requested review from spabba17

@rocrisp rocrisp Awaiting requested review from rocrisp

@bcrochet bcrochet Awaiting requested review from bcrochet

@yashoza19 yashoza19 Awaiting requested review from yashoza19

Assignees

@acmenezes acmenezes

@skattoju skattoju

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement Debug level logging as a flag Improve logger for User and Dev cases
7 participants
@yuriolisa @mrhillsman @acmenezes @bcrochet @yashoza19 @skattoju @opdevbot