-
Notifications
You must be signed in to change notification settings - Fork 329
Fix the appening deprecating warning being fatal #3605
Conversation
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.
Looks good! Returning ValidationResults separately makes sense to me. One suggestion to update to the new softer wording as discussed with @briancain, but happy to discuss it.
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.
General comment on the strong wording that we will remove multi-apps things. nvm it was updated while I was typing that comment lol
Other nit about Errors()
vs. HasErrors()
but overall LGTM
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.
Looks good! And fixes up the hard failure for me in my testing 👍🏻
Co-authored-by: Izaak Lauer <[email protected]>
Fixes #3601