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

Migrate logging to use RPC Logging methods #99

Closed
hanzei opened this issue Mar 9, 2020 · 2 comments · Fixed by #102
Closed

Migrate logging to use RPC Logging methods #99

hanzei opened this issue Mar 9, 2020 · 2 comments · Fixed by #102
Assignees
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/Go

Comments

@hanzei
Copy link
Contributor

hanzei commented Mar 9, 2020

Instead of using mlog all log calls should use the RPC methods LogX, e.g. LogDebug. This allows the server to gradually select which log messages to show in the server logs.

@hanzei hanzei added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core committer 4: Reviews Complete All reviewers have approved the pull request 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/Go Up For Grabs Ready for help from the community. Removed when someone volunteers and removed 1: PM Review Requires review by a product manager 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester 4: Reviews Complete All reviewers have approved the pull request labels Mar 9, 2020
@hanzei hanzei changed the title Migrate logging to use RPC calls Migrate logging to use RPC Logging methods Mar 9, 2020
@mo2menelzeiny
Copy link
Contributor

mo2menelzeiny commented Mar 22, 2020

@hanzei, I'm giving this a shot.
I don't know if this is the right place, but I'm having trouble with the testing.
Does any test cases cover the thrown errors? or do I have to manually test this?

My attempt for example

if err != nil {
        mlog.Error(fmt.Sprintf("Error creating autolinker: %+v: %v", c.Links[i], err))
}

I'm going to replace it with

if err != nil {
	p.API.LogError(fmt.Sprintf("Error creating autolinker: %+v: %v", c.Links[i], err))
}

do I even need to validate?

@hanzei hanzei removed the Up For Grabs Ready for help from the community. Removed when someone volunteers label Mar 23, 2020
@hanzei
Copy link
Contributor Author

hanzei commented Mar 23, 2020

@mo2menelzeiny Please see my response in your PR: #102 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Difficulty/1:Easy Easy ticket Good First Issue Suitable for first-time contributors Help Wanted Community help wanted Tech/Go
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants