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

[Pre-Commit] Handle API Modules #4175

Merged
merged 17 commits into from
Mar 27, 2024
Merged

[Pre-Commit] Handle API Modules #4175

merged 17 commits into from
Mar 27, 2024

Conversation

ilaner
Copy link
Contributor

@ilaner ilaner commented Mar 25, 2024

When Api Module is changed, it will trigger all the used integrations by the module to lint.
In addition, it will add the API modules to the used integration files, so they will be linted by the environment of the integration that uses the module.

fixes: https://jira-dc.paloaltonetworks.com/browse/CIAC-7447

@ilaner ilaner requested review from GuyAfik and JudahSchwartz March 25, 2024 12:13
GuyAfik
GuyAfik reviewed Mar 25, 2024
View reviewed changes
Copy link
Contributor

@GuyAfik GuyAfik left a comment

Choose a reason for hiding this comment

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

Amazing job!

If possible lets add a test for it

demisto_sdk/commands/pre_commit/pre_commit_command.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/pre_commit_command.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/pre_commit_command.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/hooks/docker.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/pre_commit_command.py Outdated Show resolved Hide resolved
@ilaner ilaner requested a review from dantavori as a code owner March 25, 2024 12:42
ilaner
ilaner commented Mar 25, 2024
View reviewed changes
Copy link
Contributor Author

@ilaner ilaner left a comment

Choose a reason for hiding this comment

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

but won't it crash the code if type isn't as expected? do we want it to crash?

it has to be a Script. If it is not a script there is a bug in the graph, and we want it to crash

@ilaner ilaner requested a review from GuyAfik March 25, 2024 15:35
ilaner
ilaner commented Mar 25, 2024
View reviewed changes
Copy link
Contributor Author

@ilaner ilaner left a comment

Choose a reason for hiding this comment

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

added a unit test

GuyAfik
GuyAfik approved these changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@GuyAfik GuyAfik left a comment

Choose a reason for hiding this comment

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

Good job

@GuyAfik
Copy link
Contributor

GuyAfik commented Mar 25, 2024

add changelog

@github-actions GitHub Actions
Copy link

Changelog(s) in markdown:

  • Add handling for API Modules in pre-commit. #4175

@github-actions GitHub Actions
Copy link

Changelog(s) in markdown:

  • Added handling for API Modules in pre-commit. #4175

@ilaner ilaner merged commit 7b6ae76 into master Mar 27, 2024
23 checks passed
@ilaner ilaner deleted the handle_api_modules_precommit branch March 27, 2024 15:16
This was referenced Apr 1, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuyAfik GuyAfik GuyAfik approved these changes

@JudahSchwartz JudahSchwartz Awaiting requested review from JudahSchwartz

@dantavori dantavori Awaiting requested review from dantavori dantavori is a code owner

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

Successfully merging this pull request may close these issues.
2 participants
@ilaner @GuyAfik