Skip to content
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

Lint some tools #3468

Merged
merged 3 commits into from Jan 24, 2017

Conversation

Projects
None yet
3 participants
@nsoranzo
Copy link
Member

commented Jan 23, 2017

Lint a number of tools with enhancements like:

  • drop deprecated interpreter attribute of <command>
  • use format_source instead of format in <data>
  • single-quote variables referring to paths and text params in <command>
  • add <citation>
  • fix various syntax errors.

Also some fixes and enhancements for galaxy.xsd found while linting the tools.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

Looks good to me -thanks a bunch!

@bgruening
Copy link
Member

left a comment

would be nice to fix the dependencies as well, but this can also happen in a different PR

</outputs>

<requirements>
<requirement type="package">add_scores</requirement>

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 24, 2017

Member

Can/Should we depend on python here?

@@ -1,6 +1,6 @@
<tool id="Paste1" name="Paste" version="1.0.0">
<description>two files side by side</description>
<command interpreter="perl">pasteWrapper.pl $input1 $input2 $delimiter $out_file1</command>
<command>perl '$__tool_directory__/pasteWrapper.pl' '$input1' '$input2' $delimiter '$out_file1'</command>

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 24, 2017

Member

perl dependency?

</command>
<description>reads mapping against reference sequence </description>
<requirements>
<requirement type="binary">rmapper-cs</requirement>

This comment has been minimized.

Copy link
@bgruening

bgruening Jan 24, 2017

Member

a potential candidate for the dep-mapping file

@nsoranzo

This comment has been minimized.

Copy link
Member Author

commented Jan 24, 2017

would be nice to fix the dependencies as well, but this can also happen in a different PR

@bgruening I'd prefer to have the requirement fixes in a separate PR, where we can fix them all at once using the same Perl/Python/whatever version across all these tools. Or when we do the final tool migration...

@nsoranzo

This comment has been minimized.

Copy link
Member Author

commented Jan 24, 2017

I've restarted the failed tests, it's all green now.

@bgruening bgruening merged commit 2ff068f into galaxyproject:dev Jan 24, 2017

4 checks passed

api test Build finished. 256 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 135 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 580 tests run, 0 skipped, 0 failed.
Details
@bgruening

This comment has been minimized.

Copy link
Member

commented Jan 24, 2017

Thanks @nsoranzo!

@nsoranzo nsoranzo deleted the nsoranzo:lint_tools branch Jan 24, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.