<p>New to Go - few months in. I use Jira at work a lot and use their API to automate and solve some gaps in their functionality that I like to use. Node had a good wrapper library for Jira and I wanted to make one for Go.</p>
<p>It's still starting out, but here it is: <a href="https://github.com/natdm/go-jira/tree/master/jira">https://github.com/natdm/go-jira/tree/master/jira</a></p>
<p>Is it pretty efficient as is? "Idiomatic"? Wanted to ask before I implement other functions.</p>
<p>Thanks!</p>
<hr/>**评论：**<br/><br/>balloonanimalfarm: <pre><p>Things look good to me on the go side, you might want to use the <code>log</code> package for error reporting though. My only comment is that these lines seem copy/pasted 5-6 times:</p>
<pre><code>url := fmt.Sprintf("%s://%s/rest/%s/%s/project",
j.Protocol, j.Host, j.Name, j.Version)
resp, err := j.request("GET", url, nil)
if err != nil {
return nil, err
}
bodyText, err := ioutil.ReadAll(resp.Body)
if err != nil {
return nil, err
}
</code></pre>
<p>with only the URL being different. You should pull that into a function.</p>
<p>Otherwise, looks great!</p></pre>natdm: <pre><p>Thanks. I actually extracted the consistent functionality to that request function.. The other bits change ever so often. They could probably be extracted as well. I'll look at it.</p>
<pre><code>//Helper function, wrapper for creating JSON requests via http.NewRequest.
func (j JiraConfig) request(method, url string, body io.Reader) (resp *http.Response, err error) {
client := &http.Client{}
req, err := http.NewRequest(method, url, nil)
if err != nil {
return nil, err
}
req.SetBasicAuth(j.Username, j.Password)
req.Header.Add("Content-Type", "application/json")
return client.Do(req)
}
</code></pre></pre>tgaz: <pre><ul>
<li>Constructing the URLs via <code>url.URL</code> would be nicer.</li>
<li>Some variable names are fairly long (<code>statusCode</code>) that could be shorter, and sometimes removed. Go is about "reducing cognitive load for the reader" (whether you buy into it or not).</li>
<li>For <code>getProjectsBy</code> the if-statement over method could use a switch statement instead.</li>
<li>For <code>request</code>, note that client.Do returns an open body reader that you should close to avoid hogging connections: <a href="https://golang.org/pkg/net/http/#Client.Do" rel="nofollow">https://golang.org/pkg/net/http/#Client.Do</a></li>
<li>For <code>EditIssue</code>, splitting the two cases into two functions will make the <code>string</code> special case clearer.</li>
<li>For the <code>Project.AvatarUrls</code>, idiomatic Go would call it <code>AvatarURLs</code>.</li>
<li>In many places you return maps. Go APIs often return slices (I assume it is because they are the simplest collection type in Go), and let the callers create indices they need. Or they would return a type that provides properly named accessors, so you would only have one <code>GetProjects</code> and it would return something that can look up either by key or ID.</li>
</ul>
<p>That's all I can think of. The rest looks Go:ey.</p></pre>natdm: <pre><p>Thanks! I'll work on that. Appreciate the tips.</p></pre>trevordixon: <pre><p><a href="https://golang.org/doc/effective_go.html#package-names" rel="nofollow">https://golang.org/doc/effective_go.html#package-names</a> says:</p>
<blockquote>
<p>By convention, packages are given lower case, single-word names; there should be no need for underscores or mixedCaps. Err on the side of brevity, since everyone using your package will be typing that name.</p>
</blockquote>
<p>In project.go, I see a call to json.Unmarshal that ignores errors. You probably ought to deal with that error.</p></pre>
