Merged
Conversation
Copilot caught a copy paste issue Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Generate genome update
|
Warning Newer version of the nf-core template is available. Your pipeline is using an old version of the nf-core template: 3.5.1. For more documentation on how to update your pipeline, please see the nf-core documentation and Synchronisation documentation. |
|
prototaxites
approved these changes
Feb 26, 2026
prototaxites
left a comment
prototaxites
left a comment
There was a problem hiding this comment.
Hi Damon, looks good to me - one little question about the version string output by one module
tests/default.nf.test.snap
Outdated
| "MAKECMAP_FA2CMAPMULTICOLOR": { | ||
| "perl": "5, version 26, subversion 2 (v5.26.2) built for x86_64-linux-thread-multi Copyright 1987-2018, Larry Wall Perl may be copied only under the terms of either the Artistic License or the GNU General Public License, which may be found in the Perl 5 source kit. Complete documentation for Perl, including FAQ lists, should be found on this system using \"man perl\" or \"perldoc perl\". If you have access to the Internet, point your browser at http://www.perl.org/, the Perl Home Page.", | ||
| "fa2cmap_multi_color.pl": "fa2cmap_multi_color.pl 7821 2018-08-09 17:30:15Z twang" | ||
| "fa2cmap_multi_color.pl": "fa2cmap_multi_color.pl 7821 2018-08-09 17:30:15Z twang", |
There was a problem hiding this comment.
Does this version string need some looking at?
Contributor
Author
There was a problem hiding this comment.
Wierd one isn't it!
I had a look, this is what is made by the script it self.
print $SUMMARY("Version:\t", '$Id: fa2cmap_multi_color.pl 7821 2018-08-09 17:30:15Z twang $', "\n");
Seeing as it's not one of ours, I don't wan't to change anything. Guess they were using build number and data as their versioning?
Contributor
Author
There was a problem hiding this comment.
It is something we are wanting to convert to re-write in python at somepoint. Just so we can get rid of perl as a dependency.
There was a problem hiding this comment.
In that case, maybe just strip out the fa2cmap_multi_color.pl part as that's already included in the version output as the tool name?
Optional though 😉
Contributor
Author
There was a problem hiding this comment.
naa that's a fair point, sorting it now.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previous PR went into strict_syntax instead of dev, my mistake.