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 functions from content #301

Merged
merged 11 commits into from
Mar 30, 2020
Merged

Add functions from content #301

merged 11 commits into from
Mar 30, 2020

Conversation

liorblob
Copy link
Contributor

Status

Ready

Related Issues

https://github.com/demisto/etc/issues/22938

Description

As part of the content repo cleanup, we're moving all the constants and tools to the SDK so there will be no inconsistencies.

@coveralls
Copy link
Collaborator

coveralls commented Mar 29, 2020

Pull Request Test Coverage Report for Build 2174

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 52.667%

Totals Coverage Status
Change from base Build 2164: 0.04%
Covered Lines: 3387
Relevant Lines: 6431

💛 - Coveralls

Copy link
Contributor

@yuvalbenshalom yuvalbenshalom left a comment

Choose a reason for hiding this comment

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

please add tests

return tags[0]


def collect_pack_content_items(pack_path):
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove it, will be moved to store_packs

return task_status, data


def input_to_list(input_data):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@liorblob
Copy link
Contributor Author

@yuvalbenshalom The only addition here is the Docker class, which is difficult to test because it uses SSH. We can choose to skip testing that for now or have a zoom about it with @IkaDemisto .

@IkaDemisto
Copy link
Contributor

IkaDemisto commented Mar 30, 2020

@liorblob do we need the Docker class in demisto-sdk? It is for internal use, mainly in test_integration. Are we planning to move it also to sdk?

@liorblob
Copy link
Contributor Author

@IkaDemisto Nope, should we move the class to that file?

@IkaDemisto
Copy link
Contributor

@IkaDemisto Nope, should we move the class to that file?

Yes, I think so.

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