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

CLI: remove step logs #3458

Merged
merged 14 commits into from
Apr 22, 2024
Merged

Conversation

zc-devs
Copy link
Contributor

@zc-devs zc-devs commented Mar 1, 2024

On top of #3451, addresses PR note

related to #1100

Not tested.

@qwerty287 qwerty287 added feature add new functionality cli labels Mar 1, 2024
@6543
Copy link
Member

6543 commented Mar 1, 2024

Well you still have to adjust the code as step is now not int64 but of type []int64 ...

So instead of:

if step != 0 {
...

use

if len(step) != 0 {
  stepID := step[0]
  ...

@zc-devs
Copy link
Contributor Author

zc-devs commented Mar 1, 2024

TLDR: Made separate library call. Should not be a breaking too.


LogsPurge(repoID, pipeline int64, step ...int64) error

Looks like it is not many step IDs, but any optional argument. So, I could make it

LogsPurge(repoID, pipeline int64, optArg ...int64) error

Next question is why are optional args integers only? Then it should be

LogsPurge(repoID, pipeline int64, optArg ...Interface{}) error

Then we should distinct that arguments somehow. So, it probably would be a map (argName:argValue).
Looks over complicated to me.

@qwerty287 qwerty287 added enhancement improve existing features and removed feature add new functionality labels Mar 5, 2024
@qwerty287 qwerty287 added this to the 2.5.0 milestone Mar 28, 2024
@qwerty287
Copy link
Contributor

@zc-devs I updated the branch and resolved the conflicts. Is it ready for review?

@zc-devs zc-devs marked this pull request as ready for review April 15, 2024 10:25
@zc-devs
Copy link
Contributor Author

zc-devs commented Apr 15, 2024

Let's try. Add build label, please.

@qwerty287 qwerty287 added the build_pr_images If set, the CI will build images for this PR and push to Dockerhub label Apr 15, 2024
@qwerty287
Copy link
Contributor

Done, triggers on next commit

@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Apr 15, 2024

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-3458.surge.sh

@qwerty287 qwerty287 modified the milestones: 2.5.0, 2.6.0 Apr 19, 2024
cli/log/log_purge.go Outdated Show resolved Hide resolved
@qwerty287 qwerty287 modified the milestones: 2.6.0, 2.5.0 Apr 22, 2024
@qwerty287
Copy link
Contributor

@zc-devs Format check fails, please run make format

@qwerty287 qwerty287 merged commit b02f2df into woodpecker-ci:main Apr 22, 2024
6 of 7 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 22, 2024
1 task
@zc-devs zc-devs deleted the 3451-cli-remove-step-logs branch April 22, 2024 19:34
@anbraten anbraten added feature add new functionality and removed enhancement improve existing features labels May 6, 2024
6543 added a commit to 6543-forks/woodpecker that referenced this pull request Sep 5, 2024
On top of woodpecker-ci#3451, addresses [PR
note](woodpecker-ci#3451 (comment))

related to woodpecker-ci#1100

Not tested.

---------

Co-authored-by: 6543 <[email protected]>
Co-authored-by: qwerty287 <[email protected]>
Co-authored-by: qwerty287 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build_pr_images If set, the CI will build images for this PR and push to Dockerhub cli feature add new functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants