I have finished refactoring <#C038XMTQWBT> and I t...
# release-review-trading
n
I have finished refactoring #C038XMTQWBT and I think that https://github.com/nf-core/genomeassembler/pull/171 is ready for review. I am currently running local tests of the full_test to check if the reporting works as intended, which is the only module that might still receive updates. Once I have solved all issues with the report, I will also upload the report somewhere, so reviewers can get an idea what comes out. Since this whole process took me almost a year, this PR covers many things that I was unable to properly isolate from the ongoing refactor work (template updates, module updates, switching to versions topic, docs update, getting rid of some local subworkflows that were single module etc..) and is probably closer to an "initial release" than to a verison bump in terms of what needs to be looked at. If you prefer, I can also close 171 and open a new PR, which does not contain previous partial reviews etc, while these offer interesting historical context and insights into my "process" I guess this information is not super relevant, probably depends a bit on how you do the review. Please let me know if you would prefer a new PR. I am currently planning on doing dev->master release once this is merged, and would probably request the same reviewers to approve the (minor) release PR. I guess the only upside here is that there is basically no need to compare to the code to the previous version, because it changed a lot. I have already asked @Jim Downie who is doing similar things in sanger-tol, and traded (admittedly an unfair trade in my favour) with @Evangelos Karatzas to review this. Thanks to both of you, but since this will inflict a large amount of work on you, please let me know if something changed and either of you cannot do this. I am of course also happy to trade with additional reviewers.
😂 1
e
It's fine, just make sure your end-to-end nf-test covers most of the stuff that should be covered without making it run forever and you should be fine. I'll try to go through ti first thing tomorrow morning
n
Do you mean the ci nf-test? That covers very basic things, but the full test covers everyhting and runs forever 😉
😂 1
e
Aws on fire!
n
The full takes about 2x longer because of a signle module (medaka) which is very slow for me, and I do not really know why
e
So how long does the test.conf take?
curious
n
Probably depends a lot on your cluster / queue, but for me ususally around 1-2 days
e
rip
So the test_full will take 4 days?!
n
Oh
sorry, lol, test.conf takes like 10 minutes in CI
test_full takes 1-2 days
e
phewwwwwwwwwwww
n
test.conf only tests very minimal the core function
test_full is more sophisticated and tests some combinations, and also assembles a full plant genome several times
e
Feels like an overkill, and AWS would probably break down in this duration and you'll never get your AWS output files for the website which will be a bummer. I've never had such long tests though so somebody else might be able to predict better if this will manage to run or not
n
I am happy to remove
medaka
from the full test, which adds around 24h
e
Fire away
n
Will do that once my local tests with test_full finished
e
maybe create another test_medaka profile for that, but not on test_full
👍 1
n
I do not need provide snapshots for the full test right?
e
no no
beer parrot 1