Skip to content

Latest commit

 

History

History
148 lines (96 loc) · 6.46 KB

guide_for_reviewers.md

File metadata and controls

148 lines (96 loc) · 6.46 KB

New Tool Review Guide

This document describes a checklist suitable as a guide for reviewers of pull requests against the IUC's tools repo. It is a guide only and reviewer discretion is still advised.

This document is a work in progress!

The comprehensive list is aimed at new tools. Obviously for tool updates just use the appropriate section of the checklist on the PR diffs.

This checklist is based on the IUC's Best Practices document.

Repository

  • Is this tool appropriate for the IUC repo (see CONTRIBUTING.md)
  • Does the tool already exist in the toolshed?
    • Is this tool warranted?
    • Does the iuc user have write access to the current repo in both Test and Main ToolSheds? (Under "Grant authority to make changes", select iuc and click "Grant access")
    • Does the IUC group have admin access to the current repo in both Test and Main ToolSheds? (Under "Repository Actions" -> "Manager repository administrators": "Intergalactic Utilities Commission" should be present in the "Groups associated with ..." box)
  • If the repository contains more than one tool, should it be separated or made a tool collection?
  • Is there a .shed.yml file?
  • Is there a tool .xml file?
  • No tool_dependencies.xml file. (Has been deprecated and is no longer "Best Practice")

Files

.shed.yml

  • Is there a correctly-formatted .shed.yml file?
  • Are the categories appropriate?
  • Does the name match the folder name and, in the case of a single tool, the tool .xml file name?
    • Alphanumeric and underscore _ only, no -
  • Is the owner set to iuc? If this is an existing tool being migrated to tools-iuc, the previous owner can be maintained in order to preserve the tool lineage, but the owner need to grant authority to make changes to iuc in the main Tool Shed repository page and add Intergalactic Utilities Commission to the admin groups in "Manager repository administrators" before the PR is merged.
  • Is there a description?
  • Are there homepage_url and remote_repository_url fields? Do they point somewhere sensible?

tool.xml

Linting

  • Does the tool pass planemo/travis lint with no warnings or errors?
  • 4 spaces indentation?

Order of XML Elements

<tool> (name and id etc.)

  • Are the id and name sensible and not previously used?
  • Does the version follow PEP 440 with +galaxyN?
  • Is there a @TOOL_VERSION@ macro token used? (Should there be?)
  • If there is a profile attribute, is it appropriate?

<description>

  • Is there a description tag?
  • Is the description of suitable length?

<macros>

  • If there is more than one tool present (tool collection), is there a macros.xml file?
  • Is it appropriate?

<edam_topics> & <edam_operations>

<[parallelism]>

<requirements>

  • Are there corresponding conda packages in the best practice channels?
  • Are they versioned correctly with @TOOL_VERSION@? (or multiple packages/docker containers with correctly described versions)

<code>

Error detection

  • Is there a <stdio /> element, or does <command /> have a detect_errors attribute, or does the tool specify a profile attribute?

<version_command>

  • Is there a version command?
  • Is it book-ended with <![CDATA[ ... ]]> tags?

<command>

  • Is it book-ended with <![CDATA[ ... ]]> tags?
  • No interpreter attribute for the <command /> element - This is deprecated.
  • Text parameters, input and output paths 'single quoted'?
  • Is the Cheetah left-aligned, indented and readable?
  • Are multiple commands joined with &&?
  • Are any extra temporary files (such as indices etc.) created in the current working directory?
  • Are parameters of type text or having optional="true" attribute checked with if str($param) before being used?

<environment_variables>

<configfiles>

<inputs> and <parameters>

General

  • Do data parameters have a format attribute containing datatypes recognised by Galaxy?
  • Are the parameter attributes in the order suggested in the Best Practices Coding Style
  • Do the argument attributes include the long form of the underlying tool parameters?

Boolean

  • Are the truevalue and falsevalue set with the underlying tool parameter?

Dynamic Options

  • Do conditional parameters use a select and not a boolean
  • Are the advanced parameters of the tool hidden with appropriate <section> tags?

<request_param_translation>

<outputs>

  • Are optional output files selected using <filter> tags (with corresponding <select> tags in the <inputs> section)

<tests>

  • Is most of the functionality of the tool tested?
  • Are the test datasets included in the test-data directory?
  • Is the test data of suitable size? (i.e. small, ideally <1MB per file)
  • Do the tests pass?
  • Is the output filtering tested using the expect_num_outputs attribute?
  • Are there unused files in test-data/
  • In the case where the tool uses built in reference data, is there a tool-data/tool_data_table_conf.xml.test file?

<help>

  • Is it book-ended with <![CDATA[ ... ]]> tags?
  • Is it correctly formatted in restructuredText?
  • Are images in the ./static/images directory?

<citations>

  • Is there a citation
    • Is it in bibtex or doi format? (doi preferred)

Data Tables

For tools that use the built in reference data and indices:

  • Is there a tool-data/*.loc.sample file?
  • Is there a tool-data/tool_data_table_conf.xml.sample file?