https://nf-co.re logo
Join Slack
Powered by
<@U039DEJ1RJS>, can we please split up the samtool...
# team-maintainers
s
@Matthias De Smet, can we please split up the samtools bump into multiple PRs in the future? You know how many tools depend on samtools sort and index especially, and it just makes it a real PITA.
m
No need to tell me how much of a PITA it is 😅
So I choose one big PITA instead of 35 small ones
s
Doesn't need to be 35 small ones, just 3
n
1 is better to keep everything in sync though. Especially since it's just a version bump
s
It isn't "just" a version bump, it is an update that cascades through half the repository
m
yeah, I do think the same
It is a mess
But I do think it should have been an easy PR to do in retrospect, something went clearly wrong there. It is no Matthias' fault there, he just started what should have been a long hike, not a hike + cycling + swimming + climbing and having to catch monkeys somehow
s
🤨 This happens every time, it is NEVER just an easy PR.
m
this one is particularly bad, I can confirm
I will split it up I think, that will simplify my life into fixing the remaining bugs
and simplify the life of who wil review
anyhow, I have now completed this PR. @Simon Pearce do you want me to split it up?
s
not if you haev got the tests to pass
m
I have the tests
just waiting on a ✅ now
s
I can review then
m
do it, do it now
s
// Somehow conda produces empty variantsMD5, so only capture filename
?
(I know that poolsnp was doing something funny on conda, I have an open PR for the topic migration that I hadn't dug into yet)
One of you should update your nf-core installation, as this was fixed at some point recently:
m
🫠
that was for poolsnp, right?
s
the edam? I don't care frankly.
but yes, looks like it
m
I do care, so thanks for being vigilant
s
not worth the polar bears of rerunning the tests
m
then le'ts MERGE
s
poolsnp is weird, it is working fine on GitHub codespaces on conda
but for some reason is giving the empty md5sum on the CI
m
yeah, I have no idea why, and I don't have time to work on that, so at least I kept what the assertion should be in a perfect world
s
you'll need to fix the meta for poolsnp as well
m
sorry, was in a meeting
thanks for doing it
s
why did it merge before the checks passed? 😐 😐
m
auto-merge
shakefist
s
yeah, but it should wait until all the nf-test has finished, @mashehu.
It is clearly waiting for the linting to finish, else it would have merged with the broken linting
(on the previous few pushes)
we only modified poolsnp since the last good test
so I'm not too worried that we've broken everything
m
probably because off the bypass list again
s
😬
but it did care about the nf-lint
m
might have just been the timing with how long the merge queue takes?
s
possible
m
I will assess poolsnp, and do a PR if necessary
s
It passed the linting, which is the thing we were changing since the tests passed
m
yeah, no issues at all
just wanted to be sure
6 Views
Open in Slack
PreviousNext
Powered by