bazel-contrib/bazel-mypy-integration

Why aspect over rule? [Question/Feature Request]

Closed this issue · 3 comments

dmadisetti commented

Cool project 🎉

It looks pretty low effort to bind this to a rule https://docs.bazel.build/versions/2.0.0/skylark/aspects.html#invoking-the-aspect-from-a-rule

Are there challenges that are you stopping you from doing this? Are there reasons to prefer strictly an aspect approach?
I'd like to run mypy when testing and not when doing a normal build. Having a mypy_test rule seems like a natural extension.

Happy to give this a stab if you're low on bandwidth.

thundergolfer commented

It's my preference, but I think you'd be completely justified in doing:

I'd like to run mypy when testing and not when doing a normal build.

Compared with the current 'Aspect-only' implementation, using a rule run at test time:

  • makes things opt-in, rather than mandatory as a mypy_test rule can be omitted by the user. Given MyPy runs fine over untyped Python, letting users opt-out means broken types can persist in the codebase
  • adds to the weight of the BUILD files in the python code, as you're now adding a mypy_test in a bunch of places. Not a big deal, but my feeling is to be more like "You bring your py_librarys and you py_binarys, and we'll do the rest.
  • Type errors will be sometimes caught earlier in the dev process, as for me build is run more frequently than test 🤷‍♂️.

Are there challenges that are you stopping you from doing this? ... Having a mypy_test rule seems like a natural extension.

No there isn't, I don't think. Dropbox already does it this way with their dbx_mypy_test rules (think it's called that). I agree that having a rule available is a natural extension.

Happy to give this a stab if you're low on bandwidth.

Would love that. I am quite low on bandwidth, and right now I'm prioritising cleaning up some of the roughness and grossness in the mypy.bzl file.

dmadisetti commented

My bazel projects are mostly not in python, but I do have some scripting. An aspect just seems like another point of failure and overhead I don't need for most of my codebase. That said I do try to practice test driven development (https://github.com/bazelbuild/bazel-watcher is greaaaat for this. Called iblaze elsewhere), so having a rule tell me where I go wrong is ideal :)

thundergolfer commented

Thanks for your work 🙂. Think this can be closed now