Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Run static analysis only after the whole package was loaded #8

Merged
merged 2 commits into from
Nov 14, 2023

Conversation

pawamoy
Copy link
Member

@pawamoy pawamoy commented Nov 13, 2023

Closes #7

  • We use the new Griffe on_package_loaded event to statically analyse things only once the whole package was loaded, to avoid the lookup errors described in Could not resolve alias when running griffe + this on Strawberry's codebase #7.
  • Code is made more consistent between the static and dynamic helpers (though the dynamic ones still need to be implemented - this can be done later).
  • Unit tests added ☔ (only for uses described in PEP 727, not the "enhanced" features yet)

@patrick91: I tested on strawberry, and the code duplication workaround isn't needed anymore.
@tiangolo : I checked FastAPI's docs with these changes, all seems good.

Don't hesitate to check for yourselves of course, this would be greatly appreciated 🙂
Unless issues are found, I plan to merge and release this tomorrow.

@pawamoy pawamoy self-assigned this Nov 13, 2023
@pawamoy pawamoy force-pushed the defer-static-analysis-to-package-loaded-event branch from d2ab5ef to b9fe9e2 Compare November 13, 2023 15:55
@pawamoy pawamoy merged commit 08be3d0 into main Nov 14, 2023
33 checks passed
@pawamoy
Copy link
Member Author

pawamoy commented Nov 14, 2023

Released as 0.2.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could not resolve alias when running griffe + this on Strawberry's codebase
1 participant