-
Notifications
You must be signed in to change notification settings - Fork 11
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
Cont working on the new menu in the backend #4623
Conversation
CodSpeed Performance ReportMerging #4623 will degrade performances by 99.81%Comparing Summary
Benchmarks breakdown
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left some small comments. Also I think Bilal mentioned a duplicate "Deployment" under the deployment section.
icon=obj.icon.value or "", | ||
order_weight=obj.order_weight.value, | ||
path=obj.path.value or "", | ||
kind=obj.get_kind(), | ||
kind=obj.kind.value or "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the MenuItem
class and the obj.kind.value or ""
it seems like kind, path and icon should perhaps be mandatory as strings but have ""
as the default value. Having said that I don't know if this would work in the frontend (.i.e. to send in a required blank value).
3b20326
to
2e7619d
Compare
2e7619d
to
d511944
Compare
- namespace: Organization | ||
name: Manufacturer | ||
label: Manufacturer | ||
kind: LocationManufacturer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schema does seem to exist in demo data right?
dev.load-infra-schema
to load both the schema and the menuThe changes in the Python SDK are being tracked in this PR opsmill/infrahub-sdk-python#73