Goblocks: an i3status replacement written in Go. Feedback welcome.

<p>Hey guys,</p> <p>I wrote an <a href="https://i3wm.org/i3status/">i3status</a> replacement in Go called <a href="https://github.com/davidscholberg/goblocks">Goblocks</a>. It uses the <a href="https://github.com/davidscholberg/go-i3barjson">Go-i3barjson</a> library to communicate with i3bar (<a href="https://www.reddit.com/r/golang/comments/4wnd3j/goi3barjson_a_go_library_that_implements_the/">which I posted about here a few weeks ago</a>).</p> <p>As with my previous post here, I&#39;m looking to get feedback from more experienced Go developers. I&#39;m fairly happy with the design overall, but the module system seems a little clumsy since there&#39;s a lot of duplicated boilerplate-type code. I have yet to come up with a satisfactory solution to that. Any feedback on that or any other aspect of Goblocks is much appreciated.</p> <p>Thank you for your time.</p> <hr/>**评论：**<br/><br/>nerr: <pre><p>I would definitely recommend adding some tests. For example, have the update functions accept an io.Reader which is a file under normal operation, but could be a byte slice in tests. This way, you can validate that your code works as expected with a variety of input. </p> <p>This is very cool and I look forward to giving it a try!</p></pre>Spirit_of_Stallman: <pre><blockquote> <p>Gobocks requires Go version 1.7+</p> </blockquote> <p>Wow. Why? :)</p></pre>dhscholb: <pre><p>1.7 is required because <a href="https://github.com/davidscholberg/go-i3barjson" rel="nofollow">go-i3barjson</a> uses the <a href="https://golang.org/pkg/encoding/json/#Encoder.SetIndent" rel="nofollow">json SetIndent method</a> for pretty printing the json encoder output, which was added in 1.7. I&#39;m conflicted about having this requirement, because I like having the pretty printing feature, but I do realize that this will limit adoption in the short term.</p></pre>saxykeyz: <pre><p>reimplementing the setIndent method could in fact save the you the trouble of having to rely on go1.7</p></pre>jrwren: <pre><p>What trouble?</p></pre>tgulacsi: <pre><p><a href="https://github.com/davidscholberg/goblocks/blob/master/goblocks.go#L14" rel="nofollow">https://github.com/davidscholberg/goblocks/blob/master/goblocks.go#L14</a> log.Fatalf would be easier, and wouldn&#39;t exit with 0 return code.</p> <p><a href="https://github.com/davidscholberg/goblocks/blob/master/lib/modules/battery.go#L48" rel="nofollow">https://github.com/davidscholberg/goblocks/blob/master/lib/modules/battery.go#L48</a> You open the file, but don&#39;t close it on error! A &#34;defer f.Close()&#34; would be enough.</p> <p>This is a recurring problem.</p></pre>ratatask: <pre><p>Your struct methods should probably take pointers as a receiver, This</p> <pre><code>func (c Battery) GetBlockIndex() int { return c.BlockIndex } </code></pre> <p>Copies the entire Battery struct unnecessarily.</p></pre>dhscholb: <pre><p>I thought that it was good practice to use pointer receivers only if the method needs to modify the struct, unless the object is too big. I guess my question is: how big is too big in this case?</p></pre>ratatask: <pre><p>I use pointer receivers almost exclusively, just to be consistent - though if the struct has a size similar to a couple of pointers, maybe it&#39;s not worth it.</p> <p>See <a href="https://github.com/golang/go/wiki/CodeReviewComments#receiver-type" rel="nofollow">https://github.com/golang/go/wiki/CodeReviewComments#receiver-type</a> if you want more guidelines.</p></pre>progfrog: <pre><p>Why is there block_index on each block in goblocks.yml? it shouldn&#39;t have to be mandatory; you can easily infer block position just by going through all blocks. Or if you want to, you can assign block_index, but it shouldn&#39;t be mandatory.</p></pre>dhscholb: <pre><p>I agree that requiring block_index is a bit awkward. The reason I have the block_index field is because of the way the config is unmarshalled. If you look at the BlockConfigs struct in <a href="https://github.com/davidscholberg/goblocks/blob/master/lib/modules/configure.go" rel="nofollow">lib/modules/configure.go</a>, each block is either a struct or a struct slice. I could, of course, define the block configs in a YAML array so that the index is inferred, but then I&#39;m not sure how to unmarshal that into the distinct block config structs.</p></pre>semi-: <pre><p>I haven&#39;t looked at your code any, but just a suggestion: could you throw some screenshots up on your github? I mean I get what your project does and it seems relevant to my interests, I might even give it a try later, but as a lazy internetter it would be helpful to get a glimpse of what the end result can look like. </p></pre>dhscholb: <pre><p>I do agree with this suggestion. I was initially hesitant to have a screenshot since the look of the end result is highly dependent on your i3bar and Goblocks config. Having said that, I do think it would be useful to have a screenshot that shows what some simple Goblocks config will look like, so I will get to that at some point.</p></pre>