Revert "Set the default line length to infinity (-1) (#571)" #670
Conversation
This reverts commit 0b1645d. Signed-off-by: Davanum Srinivas <[email protected]>
|
Why revert? Why not make it configurable, if that's what you need? |
|
If this is merged it will break all the code out there that's had work arounds removed. |
|
I would also suggest a test be added for this. |
|
@jjshoe happy to the necessary work either way (add a test or change the patch to make it configurable). If you prefer the configurable path, what would that look like? (i.e, how can we specify the setting?) |
|
It'd look something like this, IMO: #455 (note that that PR was for go-yaml.v3, not v2 - the code is pretty much the same though) |
|
+1, #571 was a breaking change and is now causing issues for updating vendored dependencies in github.com/kubernetes/kubernetes due to tests expecting the old line length. It would be great to either revert this breaking change or make it configurable. |
|
@bobbypage just as quick to update the tests over in kubernetes, no? |
|
I think there may be further reaching issues across kubernetes and it's consumers due to this change see kubernetes/kubernetes#95571 (comment). @liggitt may have some more context there. |
|
making it configurable seems useful, but preserving the default behavior of yaml.v2 prior to 0b1645d is what I would expect |
|
Either way, if v2 is deprecated, and you move up to v3, you'll need to handle the change? |
I would expect yaml.v3 to give configuration options to allow compatible output for yaml.v2 users, but yes, changing the default in yaml.v3 would be reasonable |
|
It was clearly a mistake to accept the formatting change in v2. The current situation is unfortunate, though, because if we remove the change we'll break some people twice to avoid breaking some people once. Given the size of Kubernetes both in terms of code and community, I'm willing to compromise on the side of reverting the change made to v2, but to reduce the pain of those that already made the migration towards the larger change, I'll also add an API that will enable the new behavior so that people can simply add a line to their code and not have to reformat everything again. I'll spend some time on this issue later today. |
That seems like a good outcome. Thanks for looking at it. |
|
@niemeyer sounds great! thanks for doing this. please let me know if i should close this PR. |
It was clearly a mistake to accept the default formatting change in v2, and now there's no ideal choice. Either we revert the change and break some projects twice, or we keep the change and break some projects once. Given the report comes from Kubernetes, which has a relevant community and code size, we'll revert it. At the same time, to simplify the life of those that already started migrating towards the v3 behavior, a new FutureLineWrap function is being introduced to trivially preserve the new behavior where desired. The v3 branch is not affected by this, and will retain the default non-wrapping behavior. It will also be changed soon to support per arbitrary line-wrapping for individual encoding operations. Thanks to everyone who offered code and ideas, and apologies for the trouble.
|
There it is: 7649d45 Please let me know if this looks reasonable and I'll add a version tag soon. |
looks good to me, thanks |
Consumers still run the risk of poorly behaved things in their dependency tree toggling global options, but that's a general issue not specific to yaml or this option. The only thing I can think of to add would maybe be a comment indicating that global option should generally be set by the process owner (e.g. the |
|
@niemeyer thanks a ton. +1 to "add a version tag" |
|
i'll close this out as well. |
|
Thanks for the feedback. v2.4.0 is now tagged. @liggitt That's a good point, but there's too much nuance to be able to easily provide guidance. Most proper software is split into independent packages internally, which may end up being the right place for that in some cases. Let's see what happens next. Hopefully v3 can soon take over and that issue will be history. I need to focus on the few open issues there and get it ready for a release. |
The previous breaking change in yaml.v2 v2.3.0 has been reverted, see go-yaml/yaml#670
The previous breaking change in yaml.v2 v2.3.0 has been reverted, see go-yaml/yaml#670
This reverts commit 0b1645d.
Hi,
At kubernetes, we are having a hard time with this change as it changes default behavior with no recourse to setting a custom width (to what it was before). Please see the following scenarios we hit recently:
Signed-off-by: Davanum Srinivas [email protected]