-
Notifications
You must be signed in to change notification settings - Fork 6
validate hinted attributes on subcontrollers #209
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #209 +/- ##
==========================================
+ Coverage 89.31% 89.33% +0.01%
==========================================
Files 45 45
Lines 2265 2269 +4
==========================================
+ Hits 2023 2027 +4
Misses 242 242 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
We're currently not checking for non generic alias types e.g. Might also be good to validate hinted subcontrollers, though I guess they're not usually dot-accessed. e: pushed something, setting as draft until I fix some logic with how hinting as an Attribute should work |
dcd79d6 to
ac94896
Compare
b50ee48 to
6a666e1
Compare
6a666e1 to
2ca1bb8
Compare
46868c2 to
3ccbe0f
Compare
3ccbe0f to
4cf1a12
Compare
|
I am just doing some updates to fastcs-eiger and it has some logic to check What do you think? Possibly this could be a separate PR. |
Failing fast may be nicer, validate_hinted_attributes is currently only called on FastCS.serve. Would have to figure out a good way to track which attributes have already been checked and make sure we're not maintaining the validation logic in two places |
Co-authored-by: Gary Yendell <gary.yendell@diamond.ac.uk>
467e4eb to
be79d11
Compare
|
That is true it might be more complicated. Let's get this PR merged and I will play around with the validation while I update fastcs-eiger. |
Oversight from #187