-
Notifications
You must be signed in to change notification settings - Fork 172
Namespace for sbus scale rules #1202
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
base: master
Are you sure you want to change the base?
Conversation
|
@mattgallagher92 this is ready for review |
mattgallagher92
left a comment
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.
Is this field optional? If so, this change will break any existing clients, so we should probably make the new field optional too. If it's mandatory then there can't be any existing clients so we don't have to worry about that, in which case this change is fine (but I've added some comments about some things to improve).
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.
Should one of the tests check that the value is set correctly?
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.
Do the docs need updating too?
|
@Larocceau sorry for the delay, are there any updates on the docs and testing so this can get completed? Please @ me if you need more information. |
|
Hi @ninjarobot! Sorry for the delay; things been a bit messy for us, and I've left CIT (as has most other dev staff). I did not get around this, but would be happy to wrap this up later. |
This PR closes #1188
The changes in this PR are as follows:
I have read the contributing guidelines and have completed the following:
If I haven't completed any of the tasks above, I include the reasons why here:
Below is a minimal example configuration that includes the new features, which can be used to deploy to Azure: