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

refactor: vcf2cytosure container #1159

Merged
merged 6 commits into from
May 22, 2023
Merged

refactor: vcf2cytosure container #1159

merged 6 commits into from
May 22, 2023

Conversation

khurrammaqbool
Copy link
Collaborator

This PR:

Changed: The base of the container.
Fixed: dependency versions for the installation of vcf2cytosure.

Review and tests:

  • Tests pass
  • Code review
  • New code is executed and covered by tests, and test approve

@khurrammaqbool khurrammaqbool self-assigned this May 17, 2023
@khurrammaqbool khurrammaqbool requested a review from a team May 17, 2023 16:10
@codecov
Copy link

codecov bot commented May 17, 2023 •
edited
Loading

Codecov Report

Patch and project coverage have no change.

Comparison is base (3f1cbce) 99.31% compared to head (41494be) 99.31%.

Additional details and impacted files 
@@           Coverage Diff            @@
##           develop    #1159   +/-   ##
========================================
  Coverage    99.31%   99.31%           
========================================
  Files           29       29           
  Lines         1758     1758           
========================================
  Hits          1746     1746           
  Misses          12       12
Flag Coverage Δ
unittests 99.31% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@khurrammaqbool khurrammaqbool linked an issue May 17, 2023 that may be closed by this pull request
vcf2cytosure container failed #1158
Closed
ivadym
ivadym reviewed May 22, 2023
View reviewed changes
Copy link
Contributor

@ivadym ivadym left a comment

Choose a reason for hiding this comment

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

I ran the container build again on your PR and it seems to have resolved itself:

https://github.com/Clinical-Genomics/BALSAMIC/actions/runs/5001681295/jobs/9044597000?pr=1126

@khurrammaqbool
Copy link
Collaborator Author

I ran the container build again on your PR and it seems to have resolved itself:

https://github.com/Clinical-Genomics/BALSAMIC/actions/runs/5001681295/jobs/9044597000?pr=1126

This pull request fixes the versions of the dependencies as we have started to do to build all the containers. It is similar to what is done in #1096. Additionally, I have removed conda, which will make this container more stable, reduce the size and little bit faster to build.

@khurrammaqbool khurrammaqbool changed the title fix: vcf2cytosure container refactor: vcf2cytosure container May 22, 2023
ivadym
ivadym approved these changes May 22, 2023
View reviewed changes
Copy link
Contributor

@ivadym ivadym left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Well done ⭐ Nothing major to add, just some small suggestions 👍

BALSAMIC/containers/vcf2cytosure/vcf2cytosure.yaml Outdated Show resolved Hide resolved
BALSAMIC/containers/vcf2cytosure/Dockerfile Outdated Show resolved Hide resolved
BALSAMIC/containers/vcf2cytosure/Dockerfile Outdated Show resolved Hide resolved
BALSAMIC/containers/vcf2cytosure/Dockerfile Outdated Show resolved Hide resolved
BALSAMIC/containers/vcf2cytosure/Dockerfile Outdated Show resolved Hide resolved
@khurrammaqbool
Copy link
Collaborator Author

Thanks you 🙏 @ivadym for the suggestions.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@khurrammaqbool khurrammaqbool merged commit f7012ae into develop May 22, 2023
@khurrammaqbool khurrammaqbool deleted the fix/vcf2cytosure_container branch May 22, 2023 12:30
@khurrammaqbool khurrammaqbool mentioned this pull request Sep 13, 2023
Open
6 tasks
@ivadym ivadym mentioned this pull request Nov 6, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ivadym ivadym ivadym approved these changes

Assignees

@khurrammaqbool khurrammaqbool

Labels
None yet
Projects
BALSAMIC
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

vcf2cytosure container failed
2 participants
@khurrammaqbool @ivadym