search

regro / cf-scripts

Flagship repo for cf-regro-autotick-bot
Other
53 stars 75 forks source link

Mini-migrator to add `build: error_overlinking: true` #1383

Open isuruf opened 3 years ago

isuruf commented 3 years ago

This should catch some errors like wrong blas linking.

beckermr commented 3 years ago

Some feedstocks rely on this not being enforced. I'm not in favor of adding this by default.

isuruf commented 3 years ago

I think there are only a few, and people could easily turn it off by adding false there and the bot would ignore those feedstocks.

beckermr commented 3 years ago

Do you mean the bot or the mini migrator?

Also, if you really want this on by default, why not add it to smithy and an opt-out in the conda forge config?

beckermr commented 3 years ago

I'd very much like to see statistics on how many feedstocks rely on over linking before we turn this on.

isuruf commented 3 years ago

I mean the mini-migrator.

Also, if you really want this on by default, why not add it to smithy and an opt-out in the conda forge config?

I was going to, but then builds will start failing suddenly and maintainers will not know what caused it.

I'd very much like to see statistics on how many feedstocks rely on over linking before we turn this on.

overlinking name here is a misnomer. The issue here is under specifying dependencies, and I'd bet there'll be only a handful of packages that should be underspecifying dependencies. (gsl comes to mind, but that's it)

beckermr commented 3 years ago

Right. My worry with the mini migrator is the same as smithy. If we just add it, bot prs will fail suddenly.

We should do this like we are doing matplotlib to matplotlib-base. We can send a bot pr suggesting this change. We let that go for a while and then make an announcement that the default will change. Then a while after that we switch the default in smithy.