search

nickvanw / ircx

Basic callback-driven Go IRC bot
Apache License 2.0
37 stars 13 forks source link

Shouldn't use log package #11

Closed mvdan closed 9 years ago

mvdan commented 9 years ago

If this is a library, it should not log anything. Info messages should not be there, and error messages should be returned as errors instead of logged.

nickvanw commented 9 years ago

You're right. I'll open a PR to remove the logging and try to return errors if possible! I'll ping this issue when a PR is open.

nickvanw commented 9 years ago

@mvdan Opinions on changing the API vs removing log lines? There are a few places where there should be an error returned instead of a log line, think it's worth making breaking changes to properly return an error?

I wrote this many moons ago, but I don't want to break anyone's code based on this library.

mvdan commented 9 years ago

I didn't do a PR removing them because they can't be just removed in Reconnect(). Unless you want Reconnect() to be silent and not report what's going on.

The method should probably be redone, e.g. a Reconnect() error that then the importing Go package has to run in a loop and track directly.

mvdan commented 9 years ago

You never did a stable release and people can use vendoring, so I wouldn't worry too much about breaking the API.

mvdan commented 9 years ago

To build on that - you just broke the API, so why not fix more things while you're at it :)

nickvanw commented 9 years ago

Good point, I also don't think many people are using it :wink:

mvdan commented 9 years ago

Well, I was looking around for an irc library, and started using goirc. After a while got tired of it sucking. So looked at sorcix/irc, but since it was lacking, ended up here :)

Probably more people will do the same.

mvdan commented 9 years ago

To build on the lacking thing - I was lazy to implement the whole bot, callback and other stuff myself, especially since you already did.

nickvanw commented 9 years ago

Closing this as #15 is in!