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

Generalize the mode option in pre-commit #3765

Merged
merged 29 commits into from
Nov 16, 2023

Conversation

RotemAmit
Copy link
Contributor

@RotemAmit RotemAmit commented Nov 2, 2023

Related Issues

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

Description

Support the option of having multiple modes per hook (not just nightly mode).

@coveralls
Copy link
Collaborator

coveralls commented Nov 2, 2023

Pull Request Test Coverage Report for Build 49253157-87b1-4a06-852f-41acd43a9f61

  • 43 of 45 (95.56%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.005%) to 81.132%

Changes Missing Coverage Covered Lines Changed/Added Lines %
demisto_sdk/commands/pre_commit/hooks/docker.py 7 9 77.78%
Totals Coverage Status
Change from base Build a272c94a-ad72-4e1e-8583-0f4401ce2f54: 0.005%
Covered Lines: 33255
Relevant Lines: 40989

💛 - Coveralls

@RotemAmit RotemAmit marked this pull request as ready for review November 7, 2023 10:06
demisto_sdk/commands/coverage_analyze/coverage_report.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/hooks/general_hook.py Outdated Show resolved Hide resolved
demisto_sdk/commands/pre_commit/hooks/hook.py Outdated Show resolved Hide resolved
"""
if self.mode == PreCommitModes.NIGHTLY and self.all_files:
self.base_hook["args"].append("-a")
if self.mode and self.mode.lower() == "nightly" and self.all_files:
Copy link
Contributor

Choose a reason for hiding this comment

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

this couldnt be fixed with modes?

demisto_sdk/commands/pre_commit/pre_commit_command.py Outdated Show resolved Hide resolved
@JudahSchwartz
Copy link
Contributor

I didnt review the coverage code itself

if hook.get("docker_image"):
hook.pop("docker_image")
if hook.get("config_file_arg"):
hook.pop("config_file_arg")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a test that tests the final formation of the hooks dict?

demisto_sdk/commands/pre_commit/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

great work on this 1. nice end product despite the changing requirements 🚀

@RotemAmit RotemAmit merged commit e0c602d into master Nov 16, 2023
18 checks passed
@RotemAmit RotemAmit deleted the ciac-8764/generalize_mode_option_in_pre_commit branch November 16, 2023 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants