-
Notifications
You must be signed in to change notification settings - Fork 70
Conversation
app: cockroachdb |
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.
I don't really like how we're losing comments here
I suppose there's nothing for it though
Unless it's possible to use Ethan's comment preservation code... But I really highly doubt 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.
it might actually - i'll give it a go the yaml contents get replaced by the output from kustomize so you're right it's not possible to change heh
return nil | ||
} | ||
|
||
func (l *Kustomizer) replaceOriginal(step api.Kustomize, built []postKustomizeFile) error { |
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.
I would love a test for this
It looks like it should work fine with our mocked FS for testing, right?
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.
Which exists already, and I completely missed. Right.
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.
WOW this diff is ugly
The golang code all looks fine though, and none of the integration test diffs looked like anything besides ordering changes
(Also, none of the integration tests have a rendered.yaml change, so that's good) |
What I Did
How I Did it
How to verify it
ship init
and check the diff in the UIDescription for the Changelog
Picture of a Boat (not required but encouraged)
🚢