cibuildwheel icon indicating copy to clipboard operation
cibuildwheel copied to clipboard

Split file utils into dedicated submodule

Open MusicalNinjaDad opened this issue 8 months ago • 9 comments

This is a first step towards #1857 designed to get feedback and validate the approach.

Considerations

  1. ~~Don't make a breaking change: anyone currently using from cibuildwheel.util import *, from cibuildwheel.util import foo or import cibuildwheel.util should not notice the change. This has driven the implementation of util/__init__.py~~ Based on comments below by @mayeut & @henryiii ... Expect from util import x to be explicit in future: Adjust the relevant imports within the codebase while keeping __all__ in place for cases where from utils import * is in use.
  2. Split by family: grouping functions based on the domain that they support rather than keeping functions together that rely on each other. This means we will be importing between util submodules - via from util.submodule import bar. To me it feels like it is more likely to find valid and obvious fracture plains with this approach. Also if something is worth an independent function then it most likely either expects reuse in multiple contexts, abstracts away something that doesn't belong in the original flow but belongs somewhere else, or both.
  3. Provide insights to developers in IDE: prefer docstrings to inline comments to explain the "what" as these are rendered on mouse-over etc. Use comments to explain the "why" behind specfic design / implementation decisions

Checks performed

  • pylint doesn't produce any E0611 (no-name-in-module) errors
  • pylint doesn't produce any errors including the term "import" which are related to the these changes
  • pytest unit_test passes

Relying on the pipeline to perform more extensive integration testing

MusicalNinjaDad avatar Jun 12 '24 14:06 MusicalNinjaDad