KThorellGroup / BACTpipe

BACTpipe: An assembly and annotation pipeline for bacterial genomics
https://bactpipe.readthedocs.org
MIT License
20 stars 8 forks source link

Add cpus to shovill #150

Closed abhi18av closed 3 years ago

boulund commented 3 years ago

You're so quick @abhi18av ! Did you have time to test this already before merging to master?

In the future, I don't want us to put new commits directly into the master branch. I would prefer if we go via the develop branch, so we have time to update and review changes to the changelog and potentially also documentation for the changes introduced by recent commits, where needed. To make sure users can rely on BACTpipe behavior we cannot change things on the master branch without making notes of it in the changelog and bump the version number.

abhi18av commented 3 years ago

In the future, I don't want us to put new commits directly into the master branch. I would prefer if we go via the develop branch, so we have time to update and review changes to the changelog and potentially also documentation for the changes introduced by recent commits, where needed. To make sure users can rely on BACTpipe behavior we cannot change things on the master branch without making notes of it in the changelog and bump the version number.

Cool, I understand that we need to streamline - I do agree with batching the changes to develop 👍

However, I went ahead with this merge since (i) the change required was essentially one liner (ii) you graciously already fixed the issue here. I felt this is more like a hot-fix (apart from a normal release cycle), apologies if it was too quick 😅

boulund commented 3 years ago

I understand why, and while I agree this fix is kind of a hot fix, it is not really a critical one. Remember than even hotfixes need to be tested properly in a separate branch. What if that minor syntax error remained undetected and we suddenly broke BACTpipe for all users?