Add API bindings for the pipeline scheduling REST API #702
Conversation
| } | ||
|
|
||
| type ScheduleInterface interface { | ||
| Schedules(vcs, org, project string) (*[]Schedule, error) |
There was a problem hiding this comment.
❓ should we add ctx context.Context ad the first argument to all of these functions?
Will whatever is calling this ever care about us honouring timeouts or cancellations of requests that could be in-flight?
There was a problem hiding this comment.
In this case it seems that neither the CLI nor the terraform provider seem to, at least in existing code.
There was a problem hiding this comment.
Very fair, it's not worth adding in this case then
Codecov Report
@@ Coverage Diff @@
## master #702 +/- ##
==========================================
+ Coverage 26.95% 28.00% +1.04%
==========================================
Files 41 42 +1
Lines 4529 4795 +266
==========================================
+ Hits 1221 1343 +122
- Misses 3165 3267 +102
- Partials 143 185 +42
Continue to review full report at Codecov.
|
sulami
force-pushed
the
schedule-api
branch
4 times, most recently
from
ac2ffef to
244b188
Compare
There was a problem hiding this comment.
I would love to see more comments added before each func declaration. This is one of the top rated 'best practises' in Golang. (I linked a reference for you :) )
"Use comments
Using comments might seem very basic practice but it’s very helpful when there are so many developers working on the same project and for transferring knowledge as well. The comments should have a brief description stating the purpose of the business logic. In this way, any other developer going through your logic will have an idea of what a particular block does."
This is actually meant to be used in the CircleCI terraform provider, which uses the CLI as a library. As such I'm not adding any CLI-user-visible way of interacting with schedules as part of this.
This is actually meant to be used in the CircleCI terraform provider,
which uses the CLI as a library. As such I'm not adding any
CLI-user-visible way of interacting with schedules as part of this.