-
Notifications
You must be signed in to change notification settings - Fork 14
/
TODO.txt
138 lines (114 loc) · 8.13 KB
/
TODO.txt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
TODO:
[X] make repo public
[X] dockerhub create repo
[X] update readme with link to dockerhub
[X] travis credentials
[X] travis build status
[X] minimal travis build (no push)
[X] dockerignore
[X] test credentials, real tests
[X] build image on travis
[X] dockerhub credentials
[X] DOCKER_USERNAME
[X] DOCKER_ORG
[X] DOCKER_TOKEN
[X] push images (with latest) to travis
[X] update README, CONTRIBUTING, look for FIXME, TODO, WRITEME
[X] merge PR to master
[X] update CHANGELOG
[X] test new image with tag `master`
[ ] issue tag v0.3.0
[ ] update our pipelines to use dockerhub and latest
[ ] upgrade generate.py to use latest
[ ] make post on discourse
[ ] make PR for dutyfree ?
[ ] BUG: the tag is not passed correctly any more:
This is the Cogito GitHub status resource. Tag: <no value>, commit: e2d790c, build date: 2019-11-14
[ ] use task inside dockerfile taskfile
[ ] temporary: leave a way to build locally and push to our private registry. Maybe use a Concourse pipeline to mirror, as suggested by Alessandro.
[ ] protect running task-publish, it should run only if in CI probably
[ ] add a way to require e2e tests to run and fail if no env vars found
[ ] replace the error types that do not take parameters because they are not helping to diagnose
[ ] FIXME all statements of the form if gotErr != tc.wantErr
are wrong; they should be if !errors.Is()
[ ] CopyDir: FIXME longstanding bug: we apply template processing always, also if the file doesn't have the .template suffix!
] refactor helper.CopyDir() to be more composable in the transformers.
[ ] MOST IMPORTANT parametric is fine, but I would like a way to use the testdata also when not running the e2e tests. Is it possible? Let's see:
[ ] key names should be the same as the documented environment variables, but lowercase to hint that a potential transformation is happening:
COGITO_TEST_REPO_OWNER -> {{.cogito_test_repo_owner}}
COGITO_TEST_REPO_NAME -> {{.cogito_test_repo_name}}
[ ] I should have stub default values for these keys, for example
{{.cogito_test_repo_owner}} <- default_repo_owner
{{.cogito_test_repo_name}} <- default_repo_name
these stub values are used when the corresponding env variables are not set; this implies
that the tests are using mocks instead than e2e
[ ] fix the fact that I am using url = {{.repo_url}} instead of the individual variables,
because this makes it possible to use only 1 file template to handle both SSH and HTTPS
[ ] i need to carefully ask the question what am I testing? Am I testing cogito Out or am I testing GitHub status API? Or am I testing the full integration?
[ ] still need a simple, not error-prone way to make the difference between e2e and not. Maybe I could reconsider making it more explicit by putting the e2e in a separate test executable, maybe with compilation guards? I don't know
[ ] Add fake version for github tests, using httptest.NewServer as in hmsg
[ ] Add fake version for resource tests, using httptest.NewServer as in hmsg
[ ] Find a way to run also the E2E tests in the Docker build
[ ] How do I test this thing ???? How do I pass the fake server to Out() ??? Need to wrap in two levels or to make the code read an env var :-( or to always run the real test (no fake). I can pass the server via the "source" map. But fro security (avoid exfiltrating the token) I don't accept the server, I accept a flag like test_api_server boolean. If set, the api server will be hardcoded to "localhost" ?
[ ] CopyDir() the renamers can be replaced by a func (string) string
[ ] cogito.yml sample pipeline: is there a trick to add a dependency between the two jobs without having to use S3 or equivalent ? Maybe a resource like semver, but without any external storage / external dependency ???
[ ] When I switched from "repo" to "input-repo" I got a bug because I didn't change all instances of the "repo" key. Two possibilities:
1. change the strings to constants everywhere
2. return error if an expected key doesn't exist.
[ ] Taskfile: A clean target would be useful for removing the built docker images.
[ ] prepare to open source it :-)
[ ] in README explain that there is no public docker image OR take a decision if we want to provide it. It would be way better.
[ ] completely paramterize the pipeline, to be used also outside pix4d
[ ] add screenshots to the README to explain what is the context, the target_url and the description.
[ ] move the testhelper to its own repo
[ ] reduce docker image size
[ ] do we gain anything from deleting some of the packages:
apk del libc-utils alpine-baselayout alpine-keys busybox apk-tools
[ ] why among the dependecies, ofcourse wants yaml ? The resource doesn't need it, it gets a JSON object. That yaml is problematic because it seems to be the one that requires gcc. If I can get rid of it, I can maybe reach a smaller image ?
[ ] is there a way to use the busybox image (smaller) and bring on the certificates? Maybe I can use alpine to build, install the certs with apk, then copy the certs directory over to a busybox? Maybe, since this resource speaks only to github, I can even copy over only the cert of the CA of github ?
[ ] better docker experience
[ ] adding the go ldflags to the dockerfile as I did is wrong; now I rebuild way too often because docker detects that variables such as build time or commit hash have changed and decides to reinstall the packages!!! Fix this.
[ ] probably I can replace reflect.TypeOf(err) with the new errors.Is()
[ ] remove or update hmsg
[ ] is there a newline or not in the gitref, when a tag is present?
[ ] would it make sense to add error logging to sentry ?
[ ] rename package resource to package cogito !!!
[ ] package github: provide custom user agent (required by GH)
[ ] How to parse .git/ref (created by the git resource) when it contains also a tag?
.git/ref: Version reference detected and checked out. It will usually contain the commit SHA
ref, but also the detected tag name when using tag_filter.
[ ] investigate if this is a bug in path.Join() and open ticket if yes
// it adds a 3rd slash "/": Post https:///api.github.c ...
// API: POST /repos/:owner/:repo/statuses/:sha
// try also with and without the beginning / for "repos"
url := path.Join(s.server, "repos", s.owner, s.repo, "statuses", sha)
[ ] package resource: add more tests for TestIn
[ ] package resource: add more tests for TestCheck
[ ] package resource: is there something cleaner than this "struct{}{}" thing ?
mandatorySources = map[string]struct{}{
"owner_repo": struct{}{},
"access_token": struct{}{},
[ ] package resource: TestPut:
find a way to test missing repo dir due to `input:` pipeline misconfiguration
[ ] package resource: TestPut:
find a way to test mismatch between input: and repo:
[ ] package github: is it possible to return information about current rate limiting, to aid
in throubleshooting?
[ ] package github: is it possible to detect abuse rate limiting and report it, to help throubleshooting? On the other hand, this is already visible in the error message ...
[ ] extract the userid from the commit :-D and make it available optionally
[ ] add test TestGitHubStatusFake
Use the http.testing API
func TestGitHubStatusFake(t *testing.T) {
fakeAPI := "http://localhost:8888"
repoStatus := github.NewStatus(fakeAPI, ...)
}
[ ] add to TestGitHubStatusE2E(t *testing.T)
Query the API to validate that the status has been added! But to do this, I need a unique text in the description, maybe I can just store the timestamp or generate an UUID and keep it in memory?
[ ] Currently we validate that state is one of the valid values in the resource itself.
Decide what do to among the following:
- leave it there
- move it to the github package
- remove it completely, since GitHub will validate in any case
Rationale:
- since the final validation is done anycase in GitHub, what is the point of adding more code to have in any case a partial validation?
- not validating allows to stay open: if tomorrow github adds another valid state, the resoulce will still work and support the new state withouh requiring a change (yes, not very probable, but still the reasoning make sense, no?)