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

add id set validations - script using real cmds, incident type has real default PB #997

Merged
merged 8 commits into from
Jan 6, 2021

Conversation

bakatzir
Copy link
Member

@bakatzir bakatzir commented Jan 3, 2021

Status

  • Ready

Description

Aside from the added validations there was code in the id.py that was not called is_id_duplicated,is_file_has_used_id - so I deleted him.

Related Issues

fixes: https://github.com/demisto/etc/issues/29559

Must have

  • Tests
  • Documentation

@bakatzir bakatzir requested review from yaakovi and roysagi January 3, 2021 12:50
@bakatzir bakatzir self-assigned this Jan 3, 2021
@coveralls
Copy link
Collaborator

coveralls commented Jan 3, 2021

Pull Request Test Coverage Report for Build afc6eb09-ce92-4241-9e5f-e57adf3cc3cf

  • 39 of 50 (78.0%) changed or added relevant lines in 3 files are covered.
  • 5 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.2%) to 87.007%

Changes Missing Coverage Covered Lines Changed/Added Lines %
demisto_sdk/commands/validate/validate_manager.py 3 5 60.0%
demisto_sdk/commands/common/hook_validations/id.py 28 37 75.68%
Files with Coverage Reduction New Missed Lines %
demisto_sdk/commands/common/errors.py 1 94.87%
demisto_sdk/commands/validate/validate_manager.py 1 90.94%
demisto_sdk/commands/common/hook_validations/id.py 3 62.96%
Totals Coverage Status
Change from base Build c2bd67f9-4639-40ca-94dc-4e56136fb13c: 0.2%
Covered Lines: 12576
Relevant Lines: 14454

💛 - Coveralls

@bakatzir bakatzir requested a review from DeanArbel January 5, 2021 07:07
Copy link
Contributor

@yaakovi yaakovi left a comment

Choose a reason for hiding this comment

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

looks ok,
@roysagi make sure to include this validation when migrating to content objects.

@bakatzir bakatzir requested a review from roysagi January 6, 2021 08:54
@bakatzir
Copy link
Member Author

bakatzir commented Jan 6, 2021

Not merging till we will know that the content is valid: demisto/content#10766

@bakatzir
Copy link
Member Author

bakatzir commented Jan 6, 2021

@bakatzir bakatzir merged commit b791217 into master Jan 6, 2021
@bakatzir bakatzir deleted the id_set_more_validations branch January 6, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants