5
\$\begingroup\$

Background

This is the first (and only) Go program I've written so far.

My employer just formed a small team to make a prototype written in Go ready for production, so I (and everyone else on my team) am learning Go as quickly as I can. We're setting up our documentation, version control, issue tracking, etc... using Gitlab. I was reading up on best practices for organizing and tagging Github issues, thinking about how to best organize the labels for our issues, and decided to write a Go program to manage the labels in our Gitlab projects. It's a good excuse to write my first Go program! We'll be able to use this to create the same labels across all our Gitlab projects.

The gist is that this program defines labels and categories. Each category is assigned a color, and each label is assigned a category. Thus all labels in a category have the same color. I use Gitlab's REST API to create labels or update their colors.

Feedback I'm Looking For

I'll take any feedback you have! These things interest me most:

  • Errors
  • How to make my code more idiomatic
  • A good approach to add concurrency (i.e. create or update labels in parallel).
  • A good approach for testing

The Code

All my code can be found in my git project, but I'll include a slightly slimmed-down version in this question.

labelmaker/main.go

package main

import (
    "labelmaker/category"
    "labelmaker/label"
    "log"
    "os"
)

var labels = []label.Label{
    {"angular", category.Platform},
    {"bug", category.Problem},
}

func main() {
    if !label.Ready() {
        log.Println("Exiting because we don't have the current labels")
        return
    }

    for _, l := range labels {
        if l.Exists() {
            l.Update()
        } else {
            l.Create()
        }
    }
}

func init() {
    log.SetOutput(os.Stdout)
}

category/category.go

package category

type Category struct {
    Name        string
    Description string
    Color       string
}

var Problem Category = Category{
    Name:        "problems",
    Description: "Issues that make the product feel broken. High priority, especially if its present in production.",
    Color:       "#CC0033",
}

var Platform Category = Category{
    Name:        "platform",
    Description: "If the repository covers multiple parts, this is how we designate where the issue lives. (i.e. iOS and Android for cross-platform tablet app).",
    Color:       "#A295D6",
}

label/label.go

package label

import (
    "labelmaker/category"
    "labelmaker/gitlab"
)

type Label struct {
    Name        string
    Category    category.Category
}

func (l *Label) Exists() bool {
    return gitlab.Exists(l.Name)
}

func (l *Label) Update() {
    gitlab.Update(l.Name, l.Category.Color)
}

func (l *Label) Create() {
    gitlab.Create(l.Name, l.Category.Color)
}

func Ready() bool {
    return gitlab.Labels != nil
}

gitlab/gitlab.go

package gitlab

import (
    "bytes"
    "encoding/json"
    "net/http"
)

const (
    personalAccessToken string = "abc123"
    projectId           string = "1"
    labelsUrl           string = "http://localhost:8080/api/v3/projects/" + projectId + "/labels?private_token=" + personalAccessToken
)

type Label struct {
    Name                      string
    Color                     string
    Description               string
    Open_issues_count         int
    Closed_issues_count       int
    Open_merge_requests_count int
    Subscribed                bool
}

var Labels map[string]Label

func Exists(name string) bool {
    _, ok := Labels[name]
    return ok
}

func Create(name string, color string) error {
    return send(name, color, "POST")
}

func Update(name string, color string) error {
    return send(name, color, "PUT")
}

func send(name string, color string, method string) (err error) {
    body := map[string]string{
        "name":  name,
        "color": color,
    }

    buff := new(bytes.Buffer)
    json.NewEncoder(buff).Encode(body)

    var req *http.Request
    req, err = http.NewRequest(method, labelsUrl, buff)
    if err != nil {
        return
    }
    req.Header.Set("Content-Type", "application/json; charset=utf-8")

    client := &http.Client{}
    var resp *http.Response
    resp, err = client.Do(req)
    if resp != nil {
        defer resp.Body.Close()
    }
    if err != nil {
        return
    }

    var labelData Label
    err = json.NewDecoder(resp.Body).Decode(&labelData)

    if err == nil {
        Labels[labelData.Name] = labelData
    }

    return
}

func fetch() (labels map[string]Label, err error) {
    labels = make(map[string]Label)

    var resp *http.Response
    resp, err = http.Get(labelsUrl)

    if resp != nil {
        defer resp.Body.Close()
    }

    if err != nil {
        Labels = nil
        return
    }

    var labelData []Label
    err = json.NewDecoder(resp.Body).Decode(&labelData)

    if err == nil {
        for _, l := range labelData {
            labels[l.Name] = l
        }
    }

    return
}

func init() {
    Labels, _ = fetch()
}
\$\endgroup\$
2
  • \$\begingroup\$ Any chance of follow up question on this one? \$\endgroup\$ Commented Aug 15, 2018 at 7:52
  • \$\begingroup\$ I don't have any questions. \$\endgroup\$ Commented Aug 16, 2018 at 11:53

1 Answer 1

1
\$\begingroup\$

Don't overuse var.

var resp *http.Response
resp, err = http.Get(labelsUrl)

// or

resp, err := http.Get(labelsUrl)

Stick to the latter, short variable declaration. No need to use var unless you really need one.

Here we must use var because it is package level block.

var Problem Category = Category{
    Name:        "problems",
    Description: "Issues that make the product feel broken. High priority, especially if its present in production.",
    Color:       "#CC0033",
}

// or

var Problem = Categoty{ // <-- no type here
    Name:        "problems",
    Description: "Issues that make the product feel broken. High priority, especially if its present in production.",
    Color:       "#CC0033",
}

If we have multiple definitions we may use var as a block:

var (
    Problem = Category{
        Name:        "problems",
        Description: "Issues that make the product feel broken. High priority, especially if its present in production.",
        Color:       "#CC0033",
    }

    Platform = Category{
        Name:        "platform",
        Description: "If the repository covers multiple parts, this is how we designate where the issue lives. (i.e. iOS and Android for cross-platform tablet app).",
        Color:       "#A295D6",
    }
)

var blocks behave simulary to import, const, and type blocks. No need to repeat your self.

init is not a good place to do an HTTP request. It will block all other code and main for rather long time until it succeeds or fails for reasons. Also it's important to handle possible networking errors which you've ignored with _.

It's nice that you've used streaming API instead of general Marshal/Unmarshal calls which may get very memory hungry.

Consider using os.Exit(1) or log.Fatal* functions to terminate process with non-zero return code. panic will do the thing as well.

Fields from struct Label doesn't follow Go naming conventions. If you need to handle different JSON names use field tags.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.