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

Move mock_logger from testutil to logger #582

Merged
merged 8 commits into from
May 10, 2024
Merged

Conversation

srijan-27
Copy link
Contributor

@srijan-27 srijan-27 commented May 8, 2024

  • mocklogger was present as part of testutil package which isn't ideal.
  • Hence, moved mocklogger in logging package itself.
  • Although to fix the cyclic dependency between logging and service packages, have created a new sub package remotelogger.
    NOTE: NewRemoteLogger is renamed to New, since now it is part of remotelogger package.
  • Closes issue: MockLogger should be in logging package and not in testutil #540

Checklist:

  • I have formatted my code using goimport and golangci-lint.
  • All new code is covered by unit tests.
  • This PR does not decrease the overall code coverage.
  • I have reviewed the code comments and documentation for clarity.

Copy link
Contributor

@vikash vikash left a comment

Choose a reason for hiding this comment

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

Shifting the mockLogger to a sub package does not really change anything. Log level for examples are not the same.

Copy link
Member

@aryanmehrotra aryanmehrotra left a comment

Choose a reason for hiding this comment

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

We can see separately if we even want to use MockLogger, as NewLogger should also be good to use!

pkg/gofr/logging/remotelogger/dynamicLevelLogger.go Outdated Show resolved Hide resolved
@srijan-27 srijan-27 requested a review from aryanmehrotra May 10, 2024 12:17
@srijan-27 srijan-27 requested a review from vikash May 10, 2024 13:03
@vikash vikash merged commit 4918573 into development May 10, 2024
13 checks passed
@vikash vikash deleted the fix/mock-logger branch May 10, 2024 19:07
@srijan-27 srijan-27 mentioned this pull request May 12, 2024
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.

4 participants