Nice. That is a really good idea and I think the implementation looks reasonable too. It is a bit drafty still, though (see comments below). Was this the first time you did something with go? How did you like it?
Some comments: - func NewIRCBridge() IRCBridge should read its configuration from the same json file where we read the data from right now too (see main.go). - I think the design is a little fragile - if something goes wrong with the IRC connection the whole metaserver comes down. How about running the IRC bridge in a go routine that gets a (buffered) channel of stuff to send and also sends stuff into a channel that the metaserver then displays? That way if the bridge is disconnected it can just discard the messages and the whole design is a bit decoupled. Also you can fake some IRC messages in a test and test the whole thing. - +func (s *Server) Motd() string - I do not agree with this change. When a method is not changing a struct, it should not be a pointer receiver. There are only a few exceptions to this rule as far as I understood go so far (not an expert by any means). - Please add some tests. Feel free to add end to end tests, there are already a few in the metaserver. -- https://code.launchpad.net/~widelands-dev/widelands-metaserver/ircbridge/+merge/203277 Your team Widelands Developers is subscribed to branch lp:widelands-metaserver. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : [email protected] Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
