Skip to content

Conversation

@ellendejong
Copy link
Collaborator

What has changed?

  • Requirements.txt removed and instead use poetry exclusively for tools CheckQC, GenderCheck, MosaicHunter.
  • Use poetry in Dockerfile.

@rernst rernst self-requested a review November 4, 2024 09:41
Copy link
Member

@rernst rernst left a comment

Choose a reason for hiding this comment

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

Volgens mij kunnen we voorkomen een dubbele venv te gebruiken? Ook interessant stukje over voorkomen van dependencies bouwen in: https://medium.com/@albertazzir/blazing-fast-python-docker-builds-with-poetry-a78a66f5aed0

@ellendejong
Copy link
Collaborator Author

Volgens mij kunnen we voorkomen een dubbele venv te gebruiken? Ook interessant stukje over voorkomen van dependencies bouwen in: https://medium.com/@albertazzir/blazing-fast-python-docker-builds-with-poetry-a78a66f5aed0

Waar verwijs je exact naar?

De --without dev kan alleen in dockerfiles, omdat voor de github actions pytest nodig is.

@rernst
Copy link
Member

rernst commented Nov 11, 2024

Volgens mij kunnen we voorkomen een dubbele venv te gebruiken? Ook interessant stukje over voorkomen van dependencies bouwen in: https://medium.com/@albertazzir/blazing-fast-python-docker-builds-with-poetry-a78a66f5aed0

Waar verwijs je exact naar?

De --without dev kan alleen in dockerfiles, omdat voor de github actions pytest nodig is.

Naar '4. Installing dependencies before copying code.' De truuk is volgens mij eerst dependencies installeren en daarna pas t package zelf. Doordat het in 2 stappen is verdeeld kan docker dingen cachen. Anyway behoorlijke optimalisatie stap, kijk maar of je het leuk vindt om in te duiken, mag ook zo door ;).

@rernst rernst self-requested a review November 11, 2024 11:01
@ellendejong ellendejong merged commit e23aa0d into feature/add_github_actions_per_tool Nov 11, 2024
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.

3 participants