woodpecker
woodpecker copied to clipboard
Add PipelineListsOptions to woodpecker-go
I fear that's a breaking change... It's a bit pain IMO that we can not version the go lib independent of the server.
I don't see a (good) way to make it non-breaking. If you have a better idea, please let me know. However, as this will be breaking anyway, we should replace all other raw query parameters used in the lib (e.g. https://github.com/woodpecker-ci/woodpecker/blob/main/woodpecker-go/woodpecker/repo.go#L6) by proper opt structs as well as this makes it easier to extend with future options.
Changes:
- Add
PipelineListOptions
and expose them to cli
❯ ./dist/woodpecker-cli pipeline ls --format "#{{ .Number }} {{ .Started }}" --before "2023-08-21T21:11:02+02:00" 2
#1 1692644861
❯ ./dist/woodpecker-cli pipeline ls --format "#{{ .Number }} {{ .Started }}" --after "2023-12-11T15:07:04+01:00" 2
#43 1702762952
- Add
RepoListOptions
and expose them to cli
❯ ./dist/woodpecker-cli repo ls --all
testorg/test (id: 2, forgeRemoteID: 3, isActive: true)
testorg2/test2 (id: 4, forgeRemoteID: 4, isActive: true)
woodpecker/woodpecker-test (id: 0, forgeRemoteID: 1, isActive: false)
testorg/woodpecker-test (id: 6, forgeRemoteID: 2, isActive: true)
- Add
DeployOptions
- Add
PipelineStartOptions
- Add
PipelineLastOptions
- Add
RepoPostOptions
- Add
RepoMoveOptions
How do we handle such PRs in general? Keep it open until we want to do a major version bump?
We actually just merged them and ignored that they're breaking...
Maybe we should just move the lib to a dedicated repo? But same argumentation might apply to cli, agent etc. which would end in a repo mess instead of the current monorepo approach. So maybe a bad idea 🙃
Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-3652.surge.sh
That's correct. It's already on my to-do list thanks for the reminder :)
before we merge this, I want to have a bugfix release!
... Nobody wanted to rush through especially not because the PR is not finished and requires a major version bump...
I agree, but which bugfixes should the contained in this patch release? There's no open/merged PR currently (except some docs enhancements) and AFAIK there was nothing critical reported
I want a majour bump soon-isch to get some breaking stuff done ...
... so we can just wait till that
I dont get your point. This PR was marked as breaking all the time. Its was already planned to wait with a merge till the next breaking release all the time.
First you want a bugfix release first (I still dont know how this is related to this PR) now you want a major soonish 🤷♂️
Im (again) pretty confused about this communication style. Just dropping a single sentence and not responding to any question is pretty bad.
Current code lgtm, but you introduced a
ListOptions
type, but did not apply it to all list api methods.Missing in:
UserList
RegistryList
CronList
SecretList
@qwerty287 Was looking into it again. These client methods don't have any additional options. Should we still add an empty options type?
If we want to add empty list option types to every list method, what about:
OrgRegistryList(orgID int64) ([]*Registry, error)
GlobalRegistryList() ([]*Registry, error)
OrgSecretList(orgID int64) ([]*Secret, error)
GlobalSecretList() ([]*Secret, error)
AgentList() ([]*Agent, error)
AgentTasksList(int64) ([]*Task, error)
My comment was about the pagination options. They're not supported yet, you need to add them and also add the query params to the URL.
ci faild related