feat!: Allow NameFieldEscape configurable #855
Conversation
BREAKING CHANGE: Allow NameFieldEscape configurable and add API path builder for Http and MQTT client Signed-off-by: bruce <[email protected]>
| return &pathBuilder{} | ||
| } | ||
|
|
||
| func (b *pathBuilder) EnableNameFieldEscape(enableNameFieldEscape bool) *pathBuilder { |
There was a problem hiding this comment.
Why not pass enableNameFieldEscape bool to NewPathBuilder and eliminate this function?
There was a problem hiding this comment.
I thought it was not a required value for the pathBuilder, so I didn't put it into the construct function.
There was a problem hiding this comment.
I thought it was not a required value for the pathBuilder, so I didn't put it into the construct function.
Looks like you are always calling EnableNameFieldEscape so seems it is required.
common.NewPathBuilder().EnableNameFieldEscape
There was a problem hiding this comment.
Yes, I am always calling the function, but we will remove it in EdgeX 4.0, so would it be optional in the future?
By the way, I'm fine with removing the EnableNameFieldEscape function.
There was a problem hiding this comment.
It will be removed, so it will not exist for being optional. Also the clean-up for 4.0 will be easier (slightly) if it is a parameter to the NewPathBuilder. ;-) This not a big deal, so approving PR as is since device services are failing again without it.
BREAKING CHANGE: Allow NameFieldEscape configurable and add API path builder for Http and MQTT client
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-core-contracts/blob/main/.github/Contributing.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)Testing Instructions
New Dependency Instructions (If applicable)