nf-core
Convert ultraplex to nf-test
Convert ultraplex to nf-test, and address the no_match files not being removed by the attempted regex, as reported in #5693.
I should say thank you to @mahesh-panchal for suggesting the way to not capture the no_match files in the channel.
Nice work. I notice the
adapter_seqis an optional val. This could be removed but I don't know what this will break. I just checked the linting and the meta.yml needs updating.
Ok, I removed the adapter_seq value channel, that can be passed via args, and updated the meta. I also rewrote the code for the actual command while I was at it.
Hi guysCould you hold fire until I have a chance to review please, this module is used exclusively in my pipelines for now I think.CheersCharlotteSent from my iPhoneOn 5 Jun 2024, at 19:55, Simon Pearce @.***> wrote:
Nice work. I notice the adapter_seq is an optional val. This could be removed but I don't know what this will break. I just checked the linting and the meta.yml needs updating.
Ok, I removed the adapter_seq value channel, that can be passed via args, and updated the meta. I also rewrote the code for the actual command while I was at it.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because your review was requested.Message ID: @.***>
I'll pop it into draft to make sure it doesn't get merged before you look at it @CharlotteAnne
Yes, I can.
On Thu, 13 Jun 2024 at 11:58, Simon Pearce @.***> wrote:
@CharlotteAnne https://github.com/CharlotteAnne , are you able to take a look at the suggested changes?
— Reply to this email directly, view it on GitHub https://github.com/nf-core/modules/pull/5706#issuecomment-2165318584, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFVBH3JIAWHPWKTR3IN7ZVTZHF3PHAVCNFSM6AAAAABIMG5HLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRVGMYTQNJYGQ . You are receiving this because you were mentioned.Message ID: @.***>
@CharlotteAnne , there are bulk updates incoming to the modules repo, so I would like to get this finished off this week. Can you please take a look if you want to do so.
Hi Simon
I really appreciate the work on the module. I think the nf-core team need to be a bit considerate that not all contributors are doing Nextflow nf-core as their full-time job. I am a researcher juggling a lot of projects and am currently on Summer holiday - given this is a module I am pretty sure only I am using - why should it be a problem to wait a month to push all the updates? You can of course go ahead without my testing, but feels like it would just make more work in the long run. I think this is a general issue and also comes back to a problem that I think is that the nf-core team are too cavalier about making breaking changes in updates. It creates a lot of tech debt for scientists who are not full-time Nextflow developers (ie. most people using Nextflow/nf-core). I guess when I'm back I'll turn up to the office hours and have a grumble there :P
Cheers, Charlotte
On Wed, 19 Jun 2024 at 06:11, Simon Pearce @.***> wrote:
@CharlotteAnne https://github.com/CharlotteAnne , there are bulk updates incoming to the modules repo, so I would like to get this finished off this week. Can you please take a look if you want to do so.
— Reply to this email directly, view it on GitHub https://github.com/nf-core/modules/pull/5706#issuecomment-2178687888, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFVBH3JYPHAQA6BX5AULI3TZIF7PRAVCNFSM6AAAAABIMG5HLOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNZYGY4DOOBYHA . You are receiving this because you were mentioned.Message ID: @.***>
@CharlotteAnne , do you have some time to look at this now? For the record, I'm not doing this as my job either, and there is at least one other person trying to use the module: https://github.com/nf-core/modules/issues/5693.
Hi SimonThanks for your patience and pointing out the issue, I’ll look through this tomorrow - cheers!CharlotteSent from my iPhoneOn 20 Aug 2024, at 17:06, Simon Pearce @.***> wrote: @CharlotteAnne , do you have some time to look at this now? For the record, I'm not doing this as my job either, and there is at least one other person trying to use the module: #5693.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>
I should say I've brought this up in the maintainers meeting. We've agreed that 3 months is a reasonable time to ask a maintainer for a reasonable time to review. We're going to ask for a second review and once that's in, we'll move forward with it's inclusion.
