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

Provide generic logging interface #7

Merged
merged 1 commit into from
Oct 18, 2018
Merged

Provide generic logging interface #7

merged 1 commit into from
Oct 18, 2018

Conversation

andig
Copy link
Contributor

@andig andig commented Oct 17, 2018

Migrated from goburrow/modbus#33

@gq0 gq0 self-requested a review October 18, 2018 14:23
@gq0
Copy link
Contributor

gq0 commented Oct 18, 2018

Hi @andig, thanks for forwarding the PR.

Just to understand your intend first. You want to register the logrus Logger implementation at the library, correct? Would the following code snipped already be sufficient for your purpose?

handler := modbus.NewTCPClientHandler(address)
handler.Logger = log.New(logrus.New().Writer(), "your prefix ", log.LstdFlags) 

@andig
Copy link
Contributor Author

andig commented Oct 18, 2018

Yes it would. Thats not necessary. You can use log or don't if you don't want it. Actually, its even simpler:

handler.Logger = logrus.New()

or whatever the client wants to do.

Or I could inject any other object that implements Printf. Without changing anything really its already decoupled from golang log implementation.

Copy link
Contributor

@gq0 gq0 left a comment

Choose a reason for hiding this comment

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

I see your point here. Only Printf is required across the repo.

@gq0 gq0 merged commit c7d0f8b into grid-x:master Oct 18, 2018
@andig andig deleted the log branch December 2, 2018 12:56
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.

2 participants