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

alertFilters: Add param descriptions for ZAP API #6281

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

alertFilters: Add param descriptions for ZAP API #6281

wants to merge 1 commit into from

Conversation

kingthorin
Copy link
Member

Overview

Add param descriptions for Web API.

Checklist

  • Update help
  • Update changelog
  • Run ./gradlew spotlessApply for code formatting
  • Write tests
  • Check code coverage
  • Sign-off commits
  • Squash commits
  • Use a descriptive title

@kingthorin kingthorin force-pushed the afilters-api-desc branch 2 times, most recently from 4a89e33 to 09eaab9 Compare March 17, 2025 17:31
@kingthorin
Copy link
Member Author

Done and done.

@psiinon
Copy link
Member

psiinon commented Mar 17, 2025 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – 99639161-2343-4184-9596-283df786c132

Great job, no security vulnerabilities found in this Pull Request

@thc202
Copy link
Member

thc202 commented Mar 17, 2025

There are some setDescriptionTag calls that are now redundant.

@kingthorin
Copy link
Member Author

kingthorin commented Mar 17, 2025 •
edited
Loading

I don't think they are. Because the API lives at alertFilter (singular) while the resource keys are alertFilters (plural).

I'll double check though.

@thc202
Copy link
Member

thc202 commented Mar 17, 2025 •
edited
Loading

The prefix (getI18nPrefix) was changed to alertFilters.

@kingthorin
Copy link
Member Author

Tweaked.

@thc202
Copy link
Member

thc202 commented Mar 17, 2025 •
edited
Loading

There was one missed, I'd suggest removing the local variables too.

@kingthorin
Copy link
Member Author

Okay, I'll push that shortly ;)

@kingthorin
Copy link
Member Author

kingthorin commented Mar 17, 2025 •
edited
Loading

That simplifies things a bunch, yay!! 🥳

@thc202 thc202 changed the title alertFilters: Add param descriptions for Web API alertFilters: Add param descriptions for ZAP API Mar 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thc202 thc202 thc202 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kingthorin @psiinon @thc202