conda-forge / conda-smithy

The tool for managing conda-forge feedstocks.
https://conda-forge.org/
BSD 3-Clause "New" or "Revised" License
149 stars 176 forks source link

Refactor meta.yaml lint logic #1906

Open ytausch opened 6 months ago

ytausch commented 6 months ago

Checklist

This builds upon #1900 by refactoring the meta.yaml lints out of the ~700 lines lintify_meta_yaml function, modularizing the lint logic. While doing that, I added early returns, simplified code and fixed smaller issues.

I think a modularized logic like this is a lot more maintainable, but the path forward should definitely be using more Pydantic models for interacting with complex JSON / YAML files.

1900 needs to be merged first.

ytausch commented 6 months ago

Cc @0xbe7a

beckermr commented 6 months ago

Any public method should be assumed to be used. As I said before on another pr, unless we plan to release v4, there can be no api changes.

beckermr commented 6 months ago

Also, we should not be introducing any deprecation processes unless core is in agreement on how they work.

xhochy commented 6 months ago

Also, we should not be introducing any deprecation processes unless core is in agreement on how they work.

Where should we discuss this? Probably a new issue in this repo?

Still, I'm wondering who the users could be that depend on these functions? I would expect that they are only used inside of conda-forge and thus I would expect that the only thing to do would be to ensure that these functions are no longer used anywhere in our codebase.

beckermr commented 6 months ago

Yes a new issue is fine and then on a core call.

We're going to bust something for somebody even if we don't know about it. We use semver and that has meaning.

beckermr commented 6 months ago

Also while I'm not opposed to making the functions shorter here, the style in which these PRs are being made is making them difficult/impossible for me to review.

wolfv commented 2 months ago

hey @ytausch - I think we merged some similar refactors in #1981 - I hope you didn't mind. Do you want to check if we should still cherry-pick some changes from your branch?

ytausch commented 2 months ago

Thanks for pointing out @wolfv!

I will have a look at this again once #1919 is merged, which was the reason this is blocked altogether