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

Public pop.SetLogger is unusable because it accepts a private type #621

Closed
Tracked by #770
mpontillo opened this issue Feb 9, 2021 · 3 comments
Closed
Tracked by #770
Assignees
Labels
s: invalid This doesn't seem right
Milestone

Comments

@mpontillo
Copy link
Contributor

I was trying to get my [POP] logging to print to stderr rather than the default of stdout. I created a custom logging function with the same parameters as the original, as follows:

var popStderrLogger = func(lvl logging.Level, s string, args ...interface{}) {
...
}

Then I called the function as follows:

	pop.SetLogger(popStderrLogger)

However, this results in:

cannot use popStderrLogger (type func("github.com/gobuffalo/pop/logging".Level, string, ...interface {})) as type pop.logger in argument to pop.SetLogger

I then tried casting the function to the private type as well; I didn't expect it to work, but it shows what I believe is the true cause of the error:

	pop.SetLogger(popStderrLogger.(pop.logger))
cannot refer to unexported name pop.logger
@mpontillo
Copy link
Contributor Author

mpontillo commented Feb 9, 2021

Can the Logger type be made public so that callers can use it, or am I missing a better way to do this?

If I could simply pass in a stderr based logger to use instead of replicating the entire function, that would also work.

@fasmat
Copy link
Member

fasmat commented Nov 23, 2021

While I agree with you that the logger interface should be public (for documentation purposes and more), you should still be able to set a custom logger without any code changes.

Your popStderrLogger just seems to use the wrong type for logging.Level:

github.com/gobuffalo/pop/logging.Level

instead of

github.com/gobuffalo/pop/v5/logging.Level

@sio4 sio4 added the s: invalid This doesn't seem right label Sep 17, 2022
@sio4
Copy link
Member

sio4 commented Sep 17, 2022

(For someone who faced a similar issue, and seeing this issue)

While #622's direction could be considerable, the issue's title is not true.

SetLogger(func(logging.Level, string, ...interface{}) works fine and exporting the variable logger is not necessary. (the real reason of the issue was already explained above comment)

The proper usages are something like:

	pop.SetLogger(func(lvl logging.Level, s string, args ...interface{}) {
		fmt.Println("XXX LOG ---", lvl, s, args)
	})

or

var poplog = func(lvl logging.Level, s string, args ...interface{}) {
	fmt.Println("XXX LOG ---", lvl, s, args)
}

<...>
	pop.SetLogger(poplog)

No type assertion to pop.logger is required.

@sio4 sio4 closed this as completed Sep 17, 2022
@sio4 sio4 added this to the v6.0.7 milestone Sep 17, 2022
@sio4 sio4 mentioned this issue Sep 20, 2022
30 tasks
@sio4 sio4 self-assigned this Sep 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s: invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants