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.

Sign up for GitHub

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

Jump to bottom

Step logs removing API and Button #3451

Merged
merged 24 commits into from
Apr 14, 2024
Merged

Step logs removing API and Button #3451

merged 24 commits into from
Apr 14, 2024

Conversation

zc-devs
Copy link
Contributor

@zc-devs zc-devs commented Feb 27, 2024 •
edited by anbraten
Loading

Closes #3444
closes #1272

Admin:
Screenshot 2024-02-27 1
Screenshot 2024-02-27 2
Screenshot 2024-02-27 3

User:
Screenshot 2024-02-27 4

@qwerty287 qwerty287 added the feature add new functionality label Feb 27, 2024
@6543

This comment was marked as outdated.

Sign in to view
@woodpecker-bot
Copy link
Contributor

woodpecker-bot commented Feb 28, 2024 •
edited
Loading

Deployment of preview was torn down

@6543
Copy link
Member

6543 commented Feb 28, 2024

also as per CI :D -> go generate cmd/server/swagger.go and commit

@zc-devs
Copy link
Contributor Author

zc-devs commented Feb 28, 2024

also as per CI :D -> go generate cmd/server/swagger.go and commit

Unfortunately, nothing changed :(
go-generate-swagger.log

@zc-devs zc-devs mentioned this pull request Mar 1, 2024
Merged
@zc-devs
Copy link
Contributor Author

zc-devs commented Mar 1, 2024

usr before deleting (admin also have access to this button):

Screenshot 2024-03-01 211151

usr2 after deleteion, the same for guest:

Screenshot 2024-03-01 211525

6543
6543 approved these changes Mar 1, 2024
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good - did not test it jet

@qwerty287
Copy link
Contributor

Can you allow this also for repo push access? The endpoint to delete the whole pipeline's logs is also available with push access only.

@anbraten anbraten added the ui frontend related label Mar 5, 2024
@lafriks
Copy link
Contributor

lafriks commented Mar 20, 2024

Ideally would be if the delete button would be hidden under the sub menu under the button with horizontal ... icon or something like that (not requirement for this PR)

@xoxys xoxys mentioned this pull request Mar 21, 2024
Merged
@qwerty287 qwerty287 added this to the 2.5.0 milestone Mar 21, 2024
qwerty287
qwerty287 reviewed Mar 22, 2024
View reviewed changes
6543
6543 requested changes Mar 24, 2024
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just for UX concerns, can we add a confirmation question popup in the WebUI ?

qwerty287
qwerty287 approved these changes Mar 24, 2024
View reviewed changes
Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, working

@qwerty287 qwerty287 requested a review from 6543 March 24, 2024 14:44
@qwerty287
Copy link
Contributor

qwerty287 commented Mar 28, 2024 •
edited
Loading

@6543 you requested changes, but your concerns were addressed
@zc-devs Can you fix the linters?

@codecov Codecov
Copy link

codecov bot commented Mar 28, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 39 lines in your changes are missing coverage. Please review.

Project coverage is 35.92%. Comparing base (8e45ddd) to head (b9fc970).
Report is 4 commits behind head on main.

❗ Current head b9fc970 differs from pull request most recent head de8e1d6. Consider uploading reports for the commit de8e1d6 to get more accurate results

Files Patch % Lines
server/api/pipeline.go 0.00% 39 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3451      +/-   ##
==========================================
- Coverage   35.98%   35.92%   -0.07%     
==========================================
  Files         231      231              
  Lines       15553    15602      +49     
==========================================
+ Hits         5597     5605       +8     
- Misses       9539     9580      +41     
  Partials      417      417

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@qwerty287
Copy link
Contributor

@6543 can you take a look here? You requested changes so we cant merge without your approval

6543
6543 approved these changes Apr 14, 2024
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thanks - my UX nit is addressed ❤️ !

@6543
Copy link
Member

6543 commented Apr 14, 2024

PS: sorry for the delay

@6543 6543 enabled auto-merge (squash) April 14, 2024 23:41
@6543 6543 merged commit b8763a8 into woodpecker-ci:main Apr 14, 2024
7 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 15, 2024
Merged
1 task
qwerty287 added a commit that referenced this pull request Apr 22, 2024
@zc-devs @6543 @qwerty287
CLI: remove step logs (#3458)
b02f2df 
On top of #3451, addresses [PR
note](#3451 (comment))

related to #1100

Not tested.

---------

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: qwerty287 <qwerty287@posteo.de>
@BrewTestBot BrewTestBot mentioned this pull request Jun 1, 2024
Merged
@zc-devs zc-devs deleted the 3444-step-logs-remove branch June 24, 2024 20:44
6543 added a commit to 6543-forks/woodpecker that referenced this pull request Sep 5, 2024
@zc-devs @6543 @qwerty287
CLI: remove step logs (woodpecker-ci#3458)
d382bfa 
On top of woodpecker-ci#3451, addresses [PR
note](woodpecker-ci#3451 (comment))

related to woodpecker-ci#1100

Not tested.

---------

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: qwerty287 <qwerty287@posteo.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anbraten anbraten anbraten left review comments

@6543 6543 6543 approved these changes

@qwerty287 qwerty287 qwerty287 approved these changes

Assignees
No one assigned
Labels
feature add new functionality ui frontend related
Projects
None yet
Milestone
2.5.0
Development

Successfully merging this pull request may close these issues.

Ability to delete step's logs manually Expose log deletion for web UI too
6 participants
@zc-devs @6543 @woodpecker-bot @qwerty287 @lafriks @anbraten