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

Recipe (auto)formatting? #187

Closed
epruesse opened this issue Oct 4, 2017 · 1 comment
Closed

Recipe (auto)formatting? #187

epruesse opened this issue Oct 4, 2017 · 1 comment

Comments

@epruesse
Copy link
Member

epruesse commented Oct 4, 2017

@tomkinsc commented on Wed Oct 28 2015

I've been playing with Go lang lately, and enjoying how gofmt produces code of consistent style. As a new user of the language, the uniformity of style makes existing code and examples more accessible and easier to read.

Conda recipes are short, but I wonder if we should perhaps suggest that recipe authors autoformat their recipes using something like YAML lint. We could go so far as to automatically format (and commit) recipes upon being merged with master, though this may make the commit history a bit messy. I would also be happy to format existing recipes myself, via YAML lint or via a custom script that could be added to the repository.

YAML lint orders keys alphabetically by default (and also strips out comments), which is perhaps not best for conda recipes. If we were to autoformat recipes in another way, it may make sense to follow the ordering used in the examples given in the conda docs, since these are likely to be the ones new contributors refer to when creating new recipes:

package:
  name: pyinstrument
  version: "0.13.1"

source:
  git_rev: 0.13.1
  git_url: https://github.com/joerick/pyinstrument.git

requirements:
  build:
    - python
    - setuptools
  run:
    - python

test:
  imports:
    - pyinstrument

about:
  home: https://github.com/joerick/pyinstrument
  license: BSD
  license_file: LICENSE

What do you think? Is this a waste of effort, or would there be benefit to having bioconda recipes formatted in a consistent way?


@johanneskoester commented on Wed Oct 28 2015

I like this a lot. Maybe letting tests fail if lints are violated and encouraging to use a linter before creating the pull request is a good idea.


@daler commented on Wed Oct 28 2015

I'm personally in favor of a consistent format, but maybe this could be informal rather than enforced? For example, a lot of YAML processing tools I've used get rid of comments. In tricky recipes, comments can be really useful. Actually, now that I try YAML lint, it strips comment selectors like # [not py3k] that are needed for correct recipe behavior.


@johanneskoester commented on Wed Oct 28 2015

Yes, comments should definetly be allowed.


@tomkinsc commented on Wed Oct 28 2015

Having tests fail for invalid files sounds good, though I just tried building an invalid recipe (I omitted the colon after "requirements") and it built fine, so it seems the pyyaml parser is at least somewhat smart. Still, having a gate to ensuring yaml validity makes sense.

Preserving comments while autoformatting is tricky, and would require us to make some assumptions. For example, if you have the following:

- item2
# some comment
- item3
- item1

The items in the list above can be ordered (lexicographically, or in a prescribed order, whatever), but where do you place the comment after you sort?

- item1
- item2 # could go here
# or here
- item3 # or here?
# perhaps here?

Addressing # [not py3k] is more straightforward, since it should be on the same line (right?), but dealing with annotations is more complicated. Most yaml emitters discard comments, though ruamel.yaml appears to handle them. I'd suggest that if we have people autoformat their recipes that we should provide a script to perform the formatting.


@bgruening commented on Sun Oct 09 2016

@tomkinsc this is something for the hackathon, isn't it? I think a natural first step is to improve the skeleton builders and using the tools from conda-forge to lint recipes.


@tomkinsc commented on Wed Nov 02 2016

This should be part of https://github.com/bioconda/bioconda-utils


@jerowe commented on Wed Nov 02 2016

Conda forge has a linter. Maybe we could coopt it from there.


@jerowe commented on Wed Nov 02 2016

I think this is it

https://github.com/conda-forge/conda-forge-webservices

It doesn't look bad to implement. Someone would just need to add the github personal token of bioconda.


@daler commented on Thu Nov 03 2016

And also pay for hosting on heroku! Maybe an amazon micro instance would work?


@bgruening commented on Thu Nov 03 2016

@tomkinsc @jerowe what about #19


@jerowe commented on Fri Nov 04 2016

So there are fancy hooks so it the linting messages show up on the PR, but the hooks aren't really necessary.

https://github.com/conda-forge/conda-smithy/blob/master/conda_smithy/lint_recipe.py

The main linting is being done here (I think). You could call this as the first travis test, and exit 1 if it finds any linting. As far as I can tell this gets imported, the recipe_dir is passed to main, and if it returns results there is linting to be done.


@jerowe commented on Fri Nov 04 2016

@bgruening , I think using the anaconda verify is a good idea, but does it do linting?


@johanneskoester commented on Mon Nov 07 2016

We should do both. I also prefer a failing test instead of a message posted to the PR, so it should be fine to use lint_recipe directly.

@epruesse
Copy link
Member Author

Closing in favor of new issue #440 (shortened)

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant