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

feat: Add filter by ecosystem #184

Merged
merged 1 commit into from
May 27, 2024
Merged

feat: Add filter by ecosystem #184

merged 1 commit into from
May 27, 2024

Conversation

rndev15
Copy link
Contributor

@rndev15 rndev15 commented May 24, 2024

Good day @kunalnagar,
thank you for your work.

While in our project we have monorepository, we need the feature to run action for different ecosystems.
I made some changes and it works like a charm.

I know that it's not ready-to-merge PR, but maybe we can prepare it together? I'm not a JS developer btw :)

@rndev15 rndev15 requested a review from kunalnagar as a code owner May 24, 2024 10:49
@rndev15
Copy link
Contributor Author

rndev15 commented May 24, 2024

I tried to add ecosystem filter looking into PR with manifest filter - FYI

kunalnagar
kunalnagar approved these changes May 26, 2024
View reviewed changes
Copy link
Member

@kunalnagar kunalnagar left a comment

Choose a reason for hiding this comment

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

@romkaspb - thanks for the contribution! It looks great 🚀

  1. Do you mind posting some sample results as screenshots in the comment before you merge the PR?
  2. Do you mind updating the PR title to remove the WIP before you merge?

@rndev15
Copy link
Contributor Author

rndev15 commented May 27, 2024

Hey @kunalnagar,

  1. I can but there are no changes in output. We can change text to something like You have ${alertCount} vulnerabilities in *${repoName}* for ${ecosystem} ecosystem. What do you think?
  2. Yep I will. And I will squash commits when we will be going to merge it.

@kunalnagar
Copy link
Member

  1. We can change text to something like

I think let's keep the existing text as I see this getting longer as more fields are added. This gave me an idea for a feature -- customize what columns/fields should be shown in the alert that gets sent out. Ecosystem could be one of those fields.

And I will squash commits

Thanks. You can go ahead and merge.

@rndev15 rndev15 changed the title WIP: feat: Add filter by ecosystem feat: Add filter by ecosystem May 27, 2024
@rndev15
Copy link
Contributor Author

rndev15 commented May 27, 2024

@kunalnagar did squash and changed PR title. I don't have permissions to merge PR.

@kunalnagar kunalnagar merged commit 41d6d72 into kunalnagarco:main May 27, 2024
github-actions bot pushed a commit that referenced this pull request May 27, 2024
@semantic-release-bot
chore(release): 1.13.0
97c1737 
[skip ci]

## [1.13.0](v1.12.36...v1.13.0) (2024-05-27)

### Features

* Add filter by ecosystem ([#184](#184)) ([41d6d72](41d6d72))
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 1.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kunalnagar kunalnagar kunalnagar approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rndev15 @kunalnagar