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 multiple tools at once (command line API change) #139

Closed
peterjc opened this Issue Apr 24, 2015 · 14 comments

Comments

Projects
None yet
3 participants
@peterjc
Copy link
Contributor

peterjc commented Apr 24, 2015

Current usage:

Usage: planemo lint [OPTIONS] TOOL_PATH

e.g.

$ planemo lint --report_level warn ../pico_galaxy/tools/seq_filter_by_id
$ planemo lint --report_level warn ../pico_galaxy/tools/seq_filter_by_mapping

I would like to be able to list multiple paths in the same command, e.g.

$ planemo lint --report_level warn ../pico_galaxy/tools/seq_filter_by_id ../pico_galaxy/tools/seq_filter_by_mapping

This would then allow wildcard via the command line expansion, e.g.

$ planemo lint --report_level warn ../pico_galaxy/tools/seq_filter_by_*
@erasche

This comment has been minimized.

Copy link
Member

erasche commented Apr 24, 2015

👍, also would like to see a -r flag to mirror shed_lint

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 24, 2015

Good idea - I was thinking about that too, e.g.

$ planemo lint --report_level warn -r ../pico_galaxy/tools/

Worth a separate issue?

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 24, 2015

planemo lint is attempting to -r when you give it a directory - the fact that it doesn't is kind of a bug I think.

Update: In the sense it will lint everything in the directory - it is annoying that it is not going into subdirectories. I think this should just be the default behavior - objections?

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 24, 2015

@jmchilton But then it wouldn't match the semantics of test, maybe flagging it with -r is a good idea.

@erasche

This comment has been minimized.

Copy link
Member

erasche commented Apr 24, 2015

👍 to planemo lint /dir/ meaning planemo lint /dir/*.xml and -r being separate.

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 24, 2015

@erasche Okay - that feels correcter to me for some reason so as long as that is not confusing to a not-@jmchilton I think that is what we should do. Also allowing multiple tools/directories to be passed is a no brainier - test should work that way to do.

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 24, 2015

I'm happy to let you guys decide the API based on the existing usage patterns in the other planemo commands.

@jmchilton jmchilton added the linting label Apr 25, 2015

jmchilton added a commit to jmchilton/planemo that referenced this issue Apr 25, 2015

Add -r/--recursive options to planemo lint.
Shed lint with --tools will always work this way since I think the Tool Shed is pretty aggressive about finding tools.

Mentioning galaxyproject#139 - though this doesn't implement the multiple paths on the command-line API change.

jmchilton added a commit to jmchilton/planemo that referenced this issue Apr 25, 2015

jmchilton added a commit to jmchilton/planemo that referenced this issue Apr 25, 2015

@jmchilton jmchilton closed this Apr 27, 2015

peterjc added a commit to peterjc/pico_galaxy that referenced this issue Apr 27, 2015

Even more tool tag re-ordering via planemo lint checking
Now doing this en-mass, thanks to this enhancement:
galaxyproject/planemo#139
@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 27, 2015

Lovely - this is working nicely for me - thanks :)

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 28, 2015

I'm unclear about the intended usage of planemo lint vs planemo shed_lint, but does that also need to allow multiple PROJECT arguments to match?

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 28, 2015

shed_lint first finds "tool shed repositories" (e.g. directories with .shed.yml files) and then lints "any included tool" (right now that just means everything other the .shed.yml file but in the future it will be smarter and only lint things that would be uploaded). lint --recurisive will just lint all tools - it doesn't need to find a .shed.yml file first. I guess shed_lint will assume the directory you give it corresponds to a tool shed repository if it cannot find any .shed.yml files - so perhaps in your case they look pretty simillair. There is also differences related to the way they are displayed - shed_lint tries to show tool linting in the context of the repository with a little + it is slight. The other big difference is that shed_lint lint tool shed artifacts as well.

But yeah - shed_lint should also allow multiple inputs.

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 28, 2015

Thanks - and yes, since right now I have no .shed.yml files the two commands seemed very similar.

What does "tool shed artefacts" mean for shed_lint?

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 28, 2015

Tool Shed artifact linting means...

  • Pretty much full XSD validation of repository_dependencies.xml files (at the top of shed repositories).
  • Partial XSD validation of tool_dependencies.xml files (at the top of shed repositories).
  • Checking README files ( #89 ) ( #91 ) (at the top of shed repositories)
  • Basic checks of .shed.yml
    • is your repository name and owner valid
    • is the category extant
    • does the name match best practice (package_ for tool_dependency_definitions, etc...).

Can track progress on this at - #107

Basically the goal would be to ensure shed_create and shed_upload commands will work after linting. Obviously there is still a lot that linting is not catching - but it has caught some stuff (markdown readme files, malformed tool_dependencies.xml, mis-named repositories).

@peterjc

This comment has been minimized.

Copy link
Contributor Author

peterjc commented Apr 28, 2015

Thanks - I should be using planemo shed_lint in preference to planemo lint for broader coverage.

@jmchilton

This comment has been minimized.

Copy link
Member

jmchilton commented Apr 28, 2015

If you want all your tools covered regardless of what happens with shed_lint it might be worth separating it out into two commands:

planemo lint -r .
planemo shed_lint -r .

and run shed_lint without --tools. Up to you though - I'll try to continue to support shed_lint without .shed.yml files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment