conda-forge / conda-verify-feedstock

A conda-smithy repository for conda-verify.
BSD 3-Clause "New" or "Revised" License
0 stars 11 forks source link

conda-verify issues #6

Open isuruf opened 6 years ago

isuruf commented 6 years ago

https://github.com/conda/conda-verify/pull/38 https://github.com/conda-forge/cfitsio-feedstock/pull/7#issuecomment-370188053

isuruf commented 6 years ago

I've moved the packages to broken

CJ-Wright commented 6 years ago

Also there was an issue for lmfit https://travis-ci.org/conda-forge/lmfit-feedstock/jobs/348020588#L263 @ericdill

djsutherland commented 6 years ago

Relevant to the lmfit issue: https://github.com/conda/conda-verify/issues/26

msarahan commented 6 years ago

Since each of your issues have been more questions about what should or should not be checked, I would recommend disabling those checks, rather than removing conda-verify wholesale every time you don't agree with what it's saying. However, I have tried to use conda-verify's exclusion stuff, and conda-build is currently not handling it correctly.

From conda-verify's docs:

conda-verify conda-build/conda.recipe --ignore=C2117,2122

For conda-build, you should be able to set these in condarc:

conda_build:
  ignore_package_verify_scripts: 
    -C1115
    - C2117
    - C2122

@mandeep and I need to connect this with conda-build better. For the time being, I agree that conda-verify should be pulled.

jakirkham commented 6 years ago

Sorry Mike, @isuruf and I have been racing this weekend to get a conda-smithy RC out. We found the linter to be disruptive for this effort and so we pulled it. Not to mention it was blocking people doing releases on the weekend when CIs are less backlogged. We didn’t want people kind enough to do a release on the weekend to feel like their efforts were in vain. Also we can’t be fighting with tooling on two fronts or we won’t make it to conda-build 3. Sorry.

I get that y’all are trying to revitalize conda-verify and that’s great and am hopeful that we can upstream our own linter content, but I think there needs to be a bit more community engagement with regards to what behavior the linter promotes and a bit earlier in the process. Again understanding that would be hard when jump starting conda-verify, but it is something we are going to need to do if it is to be successful (i.e. accepted by the community).

We can discuss ways in which we control the linter. Though I think we need to do this at a global level for now. So a condarc fix would be preferred to start to gain parity. We will probably want to relax this over time and as we become comfortable with its behavior. Though we may still want to provide maintainers an option to override. If someone not already absorbed in the conda-build 3 effort wants to try the condarc approach over at build-setup, I’d say go for it.

Ultimately I think we should strive to have conda-verify messages fused into the linter web services. Particularly for messages involving recipe content. Post-build messages are probably stuck in CI. Better yet if they provide line numbers that we can use for the bot to comment on the recipe.

isuruf commented 6 years ago

@msarahan, I think the error message here was what made me pull it. Found invalid license family for when license family is not given confuses people a lot. If the error was that License family not given, then we can live with that and add license_family to all the recipes.

msarahan commented 6 years ago

I get that y’all are trying to revitalize conda-verify and that’s great and am hopeful that we can upstream our own linter content, but I think there needs to be a bit more community engagement with regards to what behavior the linter promotes and a bit earlier in the process. Again understanding that would be hard when jump starting conda-verify, but it is something we are going to need to do if it is to be successful (i.e. accepted by the community).

The auto-tick bot put up the update PR, not us. Conda-verify is something we'd really, really like to improve and work on, but it has been a back-burner project since @mandeep, our excellent intern, left us. We haven't published any conda-verify 3.0 packages on defaults, for exactly the reason that we haven't had time to deal with issues like this. This wound is self-inflicted.

Let me reiterate: pulling it is the right choice right now. In the future, when we have tested the ignoring functionality, maybe it can come back, and we can instead have discussions about which errors conda-forge should ignore.

ocefpaf commented 6 years ago

The auto-tick bot put up the update PR, not us.

And I merged with a goal in mind: reduce the gap between the conda-forge's and Continuum's Aggregated recipes. That means a lot of work ahead on both our linter, recipes, and conda-verify.

Conda-verify is something we'd really, really like to improve and work on, but it has been a back-burner project since @mandeep, our excellent intern, left us.

:cry:

We haven't published any conda-verify 3.0 packages on defaults, for exactly the reason that we haven't had time to deal with issues like this. This wound is self-inflicted.

Maybe it is time to start fixing it and eating our own dog food :stuck_out_tongue_winking_eye:

Let me reiterate: pulling it is the right choice right now. In the future, when we have tested the ignoring functionality, maybe it can come back, and we can instead have discussions about which errors conda-forge should ignore.

:+1:

And by using the "ignoring functionality" I believe you mean: a soft transition to a common set of rules that we'll enforce on both sides :wink:

jakirkham commented 6 years ago

Thanks for the reply Mike.

Agree that this was a conda-forge action. Just was not aware of it until after the damage was dealt. Maybe we need to have a policy that more than one person from @conda-forge/core should review and approve conda-* PRs. There's just too much going on behind the scenes for any one person to make these sorts of decisions without fallout (or for that matter deal with that fallout). Perhaps this should be a topic in our meeting tomorrow.

Glad to hear we are on the same page about conda-verify. Happy to discuss more what sorts of things seem reasonable and make improvements. Perhaps we can discuss some sort of opt-in functionality within conda-forge.yml with the option to skip certain lints.

Again happy to look at getting conda-verify back in the future and am sure there are some reasonable suggestions it provides for recipes that we can learn from. Just want to make sure we are not taking our eyes of the ball in terms of conda-build 3 and new compilers.

Thanks all. :)