Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: API Update Checker #404

Open
wants to merge 14 commits into
base: master
from
Open

Conversation

@MadCozBadd
Copy link
Collaborator

MadCozBadd commented Dec 20, 2019

PR Description

This tool checks for API Updates and makes appends to the updates checklist on trello if any new updates are found.

Type of change

Please delete options that are not relevant and add an x in [] as item is complete.

  • New feature (non-breaking change which adds functionality)

How has this been tested

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration and
also consider improving test coverage whilst working on a certain feature or package.

  • go test ./... -race
  • golangci-lint run
  • Manually tested to see last updated times and check strings

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation and regenerated documentation via the documentation tool
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally and on Travis with my changes
  • Any dependent changes have been merged and published in downstream modules
Copy link
Collaborator

shazbert left a comment

Nice initial work, will be needed in future. Please rename tool from "something" to "apichecker" for example.

"time"

"golang.org/x/net/html"

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

rm ln ;)

updateCardID = "5dfc54b96da13a6ac5ceca97"
updateChecklistID = "5dfc5a5377835d0ba025787a"
Comment on lines 47 to 48

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

Not being used? Maybe RM?

)

const (
path = "https://api.github.com/repos/%s/commits/master"

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

make const name more distinctive


const (
path = "https://api.github.com/repos/%s/commits/master"
file = "Updates.json"

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

lower case json file name

var verbose bool

func main() {
flag.BoolVar(&verbose, "verbose", false, "increases logging verbosity for GoCryptoTrader")

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

increases -> Increases
GoCryptoTrader -> API Update Checker

if err != nil {
return resp, err
}
meow := html.NewTokenizer(temp.Body)

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

ch variable name

case html.StartTagToken:
token := meow.Token()
isBool := token.Data == htmlData.TokenData
if isBool {

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator
if token.Data == htmlData.TokenData {
if err != nil {
return resp, err
}
log.Println(resp)

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

rm

if fillData.LabelsID != "" {
params.Set("idLabels", fillData.LabelsID)
}
log.Println(params.Encode())

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

rm

DateFormat: "2006-01-02",
RegExp: `section-v-(2\d{3}-\d{1,2}-\d{1,2})`,
Path: "https://docs.bitfinex.com/docs/changelog"}
log.Println(data.RegExp)

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 23, 2019

Collaborator

rm

WIP
@MadCozBadd MadCozBadd force-pushed the MadCozBadd:newtool2 branch from 18fe525 to e03c63a Dec 24, 2019
Copy link

codelingo bot left a comment

LGTM

Copy link
Collaborator

shazbert left a comment

more nits found, also please change folder name.

@@ -0,0 +1,357 @@
[

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 29, 2019

Collaborator

lower case filename

return nil, err
}
var data []ExchangeInfo
json.Unmarshal(byteValue, &data)

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 29, 2019

Collaborator

check error

if t.Data == htmlData.TextTokenData {
inner := tokenizer.Next()
if inner == html.TextToken {
tempStr := (string(tokenizer.Text()))

This comment has been minimized.

Copy link
@shazbert

shazbert Dec 29, 2019

Collaborator

can remove outer brackets across all these conversions

Copy link

codelingo bot left a comment

LGTM

Copy link
Collaborator

thrasher- left a comment

This is really cool, great work @MadCozBadd

Really like the direction this is heading and have a few nits on my first pass through. Some next steps include:

a) Linking up and making Trello configuration optional (as this fits for our needs, but not for others)
b) If Trello functionality is supported, you can use that entirely for persistent storage and thus always be able to know which items have been addressed and which ones are still lingering based on the Trello card list (non checked, checklist item)

pathGetAllLists = "https://api.trello.com/1/boards/%s/lists?cards=none&card_fields=all&filter=open&fields=all&key=%s&token=%s"
pathNewCard = "https://api.trello.com/1/cards?%s&key=%s&token=%s"
pathChecklists = "https://api.trello.com/1/checklists/%s/checkItems?%s&key=%s&token=%s"
apiKey = ""

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Please make these a cmd flag

pathChecklists = "https://api.trello.com/1/checklists/%s/checkItems?%s&key=%s&token=%s"
apiKey = ""
apiToken = ""
updateChecklistID = "5dfc5a5377835d0ba025787a"

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Please make this a cmd flag

flag.Parse()
updates, err := CheckUpdates(jsonFile)
if err != nil {
log.Println(err)

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Can just change this to log.Fatal(err)

}

// GetSha gets the sha of the latest commit
func GetSha(repoPath string) (ShaResponse, error) {

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

These funcs don't need to be exported

case github:
sha, err := GetSha(data[x].Data.GitHubData.Repo)
if err != nil {
return resp, err

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

It would be nice to individually report errors instead of returning an err, so the APIs which work can still be reported

}
if check {
if verbose {
log.Printf("%v exchange Already Exists\n", exchName)

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Please make this all lowercase for consistency

)

func TestCheckExistingExchanges(t *testing.T) {
_, _, err := CheckExistingExchanges("Updates", "Kraken")

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

First param is "Updates" which is an invalid filename, should be jsonFile

if err != nil {
return resp, err
}
for x := range data {

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

For super speed, we can parallelise these by using go routines

func CreateNewCheck(newCheck string) error {
params := url.Values{}
params.Set("name", newCheck)
path := fmt.Sprintf(pathChecklists, updateChecklistID, params.Encode(), apiKey, apiToken)

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Can just use + for these

if fillData.LabelsID != "" {
params.Set("idLabels", fillData.LabelsID)
}
path := fmt.Sprintf(pathNewCard, params.Encode(), apiKey, apiToken)

This comment has been minimized.

Copy link
@thrasher-

thrasher- Dec 30, 2019

Collaborator

Can just use + for these

WIP
Copy link

codelingo bot left a comment

LGTM

Copy link
Member

xtda left a comment

With the move over to regex the use of HTML Tokenizer adds increased complexity and point of failure

As you already have direct access to the body and can use regex to match strings in the original body you can use FindAllString instead of FindString to achieve the same result with less complexity and easier adaptability for future exchanges

package main

import (
	"fmt"
	"io/ioutil"
	"log"
	"net/http"
	"os"
	"regexp"
)

func main() {
	// strings that match
	DoRegexRequest(pathBTCMarkets, "^version: \\d{1}.\\d{1}.\\d{1}", "version: 3.0.0")
	DoRegexRequest(pathHitBTC, "newest version \\d{1}.\\d{1}","newest version 2.0")
	// strings that don't match
	DoRegexRequest(pathBTCMarkets, "^version: \\d{1}.\\d{1}.\\d{1}", "version: 2.5.0")
}

func DoRegexRequest(siteURL, regexMatch, checkString string) {
	response, err := http.Get(siteURL)
	if err != nil {
		log.Fatal(err)
	}
	defer response.Body.Close()

	body, err := ioutil.ReadAll(response.Body)
	if err != nil {
		fmt.Printf("Error reading HTTP body. %v", err)
		os.Exit(0)
	}

	reg, err := regexp.Compile(regexMatch)
	if err != nil {
		fmt.Println(err)
		os.Exit(0)
	}
	match := reg.FindAllString(string(body), -1)
	if match == nil {
		fmt.Println("no matches")
	} else {
		for x := range match {
			if match[x] != checkString {
				fmt.Printf("Possible API Update %v", siteURL)
			} else {
				fmt.Println(match[x])
			}
		}
	}
}
gocryptotrader/cmd/apicheck_new on  newtool2 [!?] via 🐹 v1.13.5 
➜ go run .                 
version: 3.0.0
newest version 2.0
Possible API Update https://api.btcmarkets.net/openapi/info/index.yaml%
)

func TestCheckExistingExchanges(t *testing.T) {
_, _, err := CheckExistingExchanges(jsonFile, "Kraken")

This comment has been minimized.

Copy link
@xtda

xtda Jan 5, 2020

Member

This will cause tests to also update the updates.json when running tests that will also cause failures if there is nay changes might be best to create "update_test.json" file with static results we can test against

break loop2
}
case html.StartTagToken:
new := tokenizer.Token()

This comment has been minimized.

Copy link
@xtda

xtda Jan 5, 2020

Member

Generally considered bad practice to use new as variable name due to New()'s usage as a reserved word in Go

Copy link

codelingo bot left a comment

LGTM

@MadCozBadd

This comment has been minimized.

Copy link
Collaborator Author

MadCozBadd commented Jan 7, 2020

So for the first check ill have to do it manually and ill write a function to create a checklist for every exchange for example: (name - Gemini 1, Complete) so when updates are found it can be changed to (name - Gemini 1, status - Incomplete). 1 in this case represents how many updates are due. Will have all exchanges added to trello with the same format (name - ExchangeName 1, status - complete) so the new functionality added can start working.

@MadCozBadd MadCozBadd added the review me label Jan 7, 2020
Copy link

codelingo bot left a comment

LGTM

Copy link
Collaborator

gloriousCode left a comment

This is looking good, but there's a few things that can be improved.

I think this needs a readme file. Running the application just resulted in
2020/01/08 07:51:36 common.SendHTTPGetRequest() error: HTTP status code 401
Which is not helpful at all for something which is crawling many endpoints.
Running it in verbose shows that it got stuck at some point:

2020/01/08 07:51:56 https://api.github.com/repos/Bittrex/bittrex.github.io/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/binance-exchange/binance-official-api-docs/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/Coinbene/API-SPOT-v2-Documents/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/Coinbene/API-SWAP-Documents/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/bithumb-pro/bithumb.pro-official-api-docs/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/coinut/api/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/gateio/gateapi-go/commits/master
2020/01/08 07:51:56 https://api.github.com/repos/LBank-exchange/lbank-official-api-docs/commits/master
2020/01/08 07:51:59 common.SendHTTPGetRequest() error: HTTP status code 401

This turns out to be from trello integration, but you'd never know if you just wanted to build and run the application.

"time"

"github.com/thrasher-corp/gocryptotrader/common/crypto"

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

Please remove spaces.
We format imports like

import(
go imports

non-go imports
)

Sadly VSC and Goland tend to split things

// CheckChangeLog checks the exchanges which support changelog updates.json
func CheckChangeLog(htmlData HTMLScrapingData) (string, error) {
var dataStrings []string
var dataString string

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

This is unused

}
}

func TestSomething(t *testing.T) {

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

Please rename. But it looks like this isn't testing anything, so you can remove if you're not overly fond of it

if err != nil {
t.Error(err)
}
}

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

Is there no way to test checkers like lbank?

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

Bumperino

}
for z := range a.CheckItems {
if strings.Contains(a.CheckItems[z].Name, resp[y]) {
err = UpdateCheckItem(a.CheckItems[z].ID, a.CheckItems[z].Name, a.CheckItems[z].State)

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

You should only perform these actions if you have a trello key set. Someone can still use this without utilising trello. Right now the application breaks if you don't have a key set

cmd/apichecker/apicheck.go Show resolved Hide resolved
cmd/apichecker/apicheck.go Show resolved Hide resolved
cmd/apichecker/apicheck.go Outdated Show resolved Hide resolved
errMap[data[x].Name] = err
}
if sha.ShaResp != data[x].Data.GitHubData.Sha {
data[x].Data.GitHubData.Sha = sha.ShaResp

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

It would be good to have a verbose ouput that it's checked github repos. Right now there's only htmlScrape which outputs data

}

// Add appends exchange data to updates.json for future api checks
func Add(fileName, exchName, checkType, path string, data interface{}, update bool) error {

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 7, 2020

Collaborator

fileName is unused

MadCozBadd added 2 commits Jan 8, 2020
Copy link
Collaborator

shazbert left a comment

This is coming along really nicely! Well done.

}
log.Println(errMap)
log.Printf("Following are the exchanges that require updates: %v\n", resp)
unsup, err := CheckMissingExchanges(fileName)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

err not checked

trelloKey = ""
trelloToken = ""
trelloChecklistID = "5dfc5a5377835d0ba025787a"
trelloCardID = "5dfc54b96da13a6ac5ceca97"
Comment on lines +54 to +57

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

These items can be included in the default configuration file and overwritten by the command line flags. This needs to be customisation due to other people using it for there own needs.

var resp ShaResponse
path := fmt.Sprintf(githubPath, repoPath)
if verbose {
log.Println(path)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

Print to standard output you are getting the latest commit SHA for x exchange from Github.


// CheckExistingExchanges checks if the given exchange exists
func CheckExistingExchanges(fileName, exchName string) ([]ExchangeInfo, bool, error) {
data, err := ReadFileData(fileName)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

We read from data file a lot, we should have one call to get files contents and load it into memory. Then query that memory, then commit changes at the end of this entire process. Then we can pass in our []ExchangeInfo to explicit functions. E.g. CheckExchangeExists([]ExchangeInfo) bool {}, then check for missing exchanges, then finally check updates.

var tempArray []string
for x := range data {
tempArray = append(tempArray, data[x].Name)
}
supportedExchs := exchange.Exchanges
for z := 0; z < len(supportedExchs); {
if common.StringDataContainsInsensitive(tempArray, supportedExchs[z]) {
supportedExchs = append(supportedExchs[:z], supportedExchs[z+1:]...)
continue
}
z++
}
return supportedExchs, nil
Comment on lines 129 to 141

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

Instead of this of this we can range over data then within that for loop, we can range over exchange.Exchanges. If found continue, if not append to var missingExchanges []string and then return that.


// SendHTTPRequest sends an unauthenticated HTTP request
func SendHTTPRequest(path string, result interface{}) error {
return common.SendHTTPGetRequest(path, true, false, result)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

set global verbose variable

CheckString: "2019-04-28",
Path: "https://www.okcoin.com/docs/en/#change-change"}
a, err := HTMLScrapeDefault(&data)
t.Log(a)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

rm and test return

log.Println(a[0])
t.Log(a)
Comment on lines 195 to 196

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

rm and test return

Path: "https://exmo.com/en/api/"},
},
}
Update("Exmo", finalResp, info)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

returned value needs to be checked.

@@ -155,7 +155,7 @@ func YesOrNo(input string) bool {
func SendHTTPRequest(method, urlPath string, headers map[string]string, body io.Reader) (string, error) {
result := strings.ToUpper(method)

if result != http.MethodPost && result != http.MethodGet && result != http.MethodDelete {
if result != http.MethodPost && result != http.MethodGet && result != http.MethodDelete && result != http.MethodPut {

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 9, 2020

Collaborator

This check should be done in http.NewRequestWithContext, so can probably rm this entire check for any outbound requests.

Copy link

codelingo bot left a comment

LGTM

@MadCozBadd MadCozBadd added reconstructing and removed review me labels Jan 16, 2020
@MadCozBadd MadCozBadd added review me and removed reconstructing labels Jan 17, 2020
Copy link

codelingo bot left a comment

LGTM

Copy link
Member

xtda left a comment

two small changes from me

Also with the way its setup currently setup if the API docs go down for whatever reason it will trigger a change detection

Not really sure the best way to handle outside checking response code for ones that follow at least that

Maybe we could also store a backup copy of the previous updates.json when its run (yes this means two copies but at least we have a known working backup if anything goes wrong)

log.Fatal(err)
}
} else {
if verbose {

This comment has been minimized.

Copy link
@xtda

xtda Jan 19, 2020

Member

Please remove the verbose wrap around this if you run apichecker without any params you get no output by default to let the user know what is happening

}
if areAPIKeysSet() {
// Assumption here is that if api key n token are set, then cardid and checklistid will be set too
if areAPIKeysSet() {

This comment has been minimized.

Copy link
@xtda

xtda Jan 19, 2020

Member

Remove the double check against areAPIKeysSet() here


## Current Features for apichecker

#### This tool allows for the generation of new documentation through templating

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

All this documentation needs to reflect the APIchecker software, just needs to be regenerated by the doc tool.

var resp ShaResponse
path := fmt.Sprintf(githubPath, repoPath)
if verbose {
fmt.Println(path)

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

Be more descriptive here and include that this is getting the SHA from the repository path.

func CheckExistingExchanges(fileName, exchName string, confData Config) (bool, error) {
var resp bool
for x := range confData.Exchanges {
if confData.Exchanges[x].Name == exchName {
resp = true
break
}
}
return resp, nil
}
Comment on lines 136 to 145

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

drop error return. Rm resp bool var and instead of breaking return true.

"CardID": "",
"ChecklistID": "",
Comment on lines +2 to +3

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

The defaults can be filled out here

#### This tool allows for the generation of new documentation through templating

From the `gocryptotrader/cmd/documentation/` folder, using the go command: **go run documentation.go** this will auto-generate and regenerate documentation across the **GoCryptoTrader** code base.
>Using the -v command will, ie **go run documentation.go -v** put the tool into verbose mode allowing you to see what is happening with a little more depth.

Be aware, this tool will:
- Works off a configuration JSON file located at ``gocryptotrader/cmd/documentation/`` for future use with multiple repositories.
- Automatically find the directory and file tree for the GoCryptoTrader source code and alert you of undocumented file systems which **need** to be updated.
- Automatically find the template folder tree.
- Fetch an updated contributor list and rank on pull request commit amount
- Sets up core folder docs for the root directory tree including **LICENSE** and **CONTRIBUTORS**

### config.json example

```json
{
"githubRepo": "https://api.github.com/repos/thrasher-corp/gocryptotrader", This is your current repo
"exclusionList": { This allows for excluded directories and files
"Files": null,
"Directories": [
"_templates",
".git",
"web"
]
},
"rootReadmeActive": true, allows a root directory README.md
"licenseFileActive": true, allows for a license file to be generated
"contributorFileActive": true, fetches a new contributor list
"referencePathToRepo": "../../"
}
```
### Template example
>place a new template **example_file.tmpl** located in the current gocryptotrader/cmd/documentation/ folder; when the documentation tool finishes it will give you the define template associated name e.g. ``Template not found for path ../../cmd/documentation create new template with \{\{define "cmd documentation" -\}\} TEMPLATE HERE \{\{end}}`` so you can replace the below example with ``\{\{define "cmd documentation" -}}``

```
\{\{\define "example_definition_created_by_documentation_tool" -}}
\{\{\template "header" .}}
## Current Features for {{.Name}}

#### A concise blurb about the package or tool system

+ Coding examples
import "github.com/thrasher-corp/gocryptotrader/something"
testString := "aAaAa"
upper := strings.ToUpper(testString)
// upper == "AAAAA"

{\{\template "contributions"}}
{\{\template "donations"}}
{\{\end}}
```

### ALL NEW UPDATES AND FILE SYSTEM ADDITIONS NEED A DOCUMENTATION UPDATE USING THIS TOOL OR PR MERGE REQUEST MAY BE POSTPONED.
Comment on lines +7 to +59

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

This template needs to reflect api checker workings.

@@ -0,0 +1,50 @@
# GoCryptoTrader package Tools

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

tools folder has been deprecated by cmd/ this can be RM'd

@@ -0,0 +1,57 @@
# GoCryptoTrader package Documentation

This comment has been minimized.

Copy link
@shazbert

shazbert Jan 19, 2020

Collaborator

tools folder has been deprecated by cmd/ this can be RM'd

Copy link
Collaborator

gloriousCode left a comment

Thanks for making those other changes! Here are a few more 🏎


// areAPIKeysSet checks if api keys and tokens are set
func areAPIKeysSet() bool {
if (trelloKey != "" && trelloKey != "key") || (trelloToken != "" && trelloToken != "token") {

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

This looks like it could all be &&. Or can you use a trello token without having an api key?

Also, you could remove the if:

return trelloKey != "" && trelloKey != "key" && trelloToken != "" && trelloToken != "token"
flag.StringVar(&apiKey, "key", "", "its an API Key for trello")
flag.StringVar(&apiToken, "token", "", "its an API Token for trello")
flag.StringVar(&updateChecklistID, "checklistid", "5dfc5a5377835d0ba025787a", "checklist id for trello")
flag.StringVar(&updateCardID, "cardid", "5dfc54b96da13a6ac5ceca97", "card id for trello")

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

You probably shouldn't actually include these keys in code. It would be better to have a config file to put them in. If you are going to, you can use your default consts that you have set above.

I'm assuming that these keys are specific keys for our own trello board

cmd/apichecker/apicheck.go Show resolved Hide resolved
default:
return "", errors.New("two or more updates were done on the same day, please check manually")
}
default:

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

No need to have a default if you do nothing with it. Golang will just skip it if it doesn't match your switch scenarios

}
}
}
default:

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

Don't need these defaults since the for loop will continue if it doesn't match the above scenarios

if err != nil {
t.Error(err)
}
}

This comment has been minimized.

Copy link
@gloriousCode

gloriousCode Jan 19, 2020

Collaborator

Bumperino

@shazbert shazbert added reconstructing and removed review me labels Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.