-
Notifications
You must be signed in to change notification settings - Fork 697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Convert ultraplex to nf-test #5706
base: master
Are you sure you want to change the base?
Conversation
I should say thank you to @mahesh-panchal for suggesting the way to not capture the no_match files in the channel. |
Co-authored-by: Mahesh Binzer-Panchal <mahesh.binzer-panchal@nbis.se>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
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 |
@CharlotteAnne , are you able to take a look at the suggested changes? |
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
<#5706 (comment)>,
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. |
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
<#5706 (comment)>,
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? |
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: ***@***.***>
|
Bump for @CharlotteAnne |
Convert ultraplex to nf-test, and address the no_match files not being removed by the attempted regex, as reported in #5693.