Skip to content
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

chore(deps): upgrade sigs.k8s.io/controller-tools to v0.14.0 #63

Merged
merged 10 commits into from
Mar 12, 2024

Conversation

justenstall
Copy link
Contributor

@justenstall justenstall commented Feb 19, 2024

This updates sigs.k8s.io/controller-tools to v0.14.0.

With this new version the rendering of multi lines and empty lines has changed a little. The asciidoc and markdown rendering engines are therefore updated to take this into account.

Closes #62

Signed-off-by: Justen Stall <justenstall@gmail.com>
Copy link

cla-checker-service bot commented Feb 19, 2024

💚 CLA has been signed

Signed-off-by: Justen Stall <justenstall@gmail.com>
@ahmetb
Copy link

ahmetb commented Feb 22, 2024

Confirming the fix works, tried it in quite a few of our repos (all of which were broken due to go1.22 -> controller-gen@v0.14 migration). Would be nice to merge soon. 😇

@justenstall
Copy link
Contributor Author

@tenstad Can you get help get this merged? test.sh failed for a whitespace difference that I can't track down

@tenstad
Copy link
Contributor

tenstad commented Feb 28, 2024

@tenstad Can you get help get this merged? test.sh failed for a whitespace difference that I can't track down

Strange diff indeed! Appears empty lines within comments is parsed differently now: tenstad@95519bd

@justenstall
Copy link
Contributor Author

@tenstad Can you get help get this merged? test.sh failed for a whitespace difference that I can't track down

Strange diff indeed! Appears empty lines within comments is parsed differently now: tenstad@95519bd

Pulled your changes and verified tests locally, should be good to go now. Thanks for the help!

In asciidoc, add hard line breaks (+) only for non-empty lines.
In markdown, add only one hard line break (</br>).
@thbkrkr
Copy link
Contributor

thbkrkr commented Feb 29, 2024

I think it's a bit more complicated because the \n are now preserved, so we need to revisit how we handle multi-line comments introduced in #53. One strange thing to me is that it looks like we now have 2 \n for empty lines?

Capture d’écran 2024-02-29 à 07 20 40

I tried something here: b4a8283.

@justenstall
Copy link
Contributor Author

@thbkrkr Should I pull your changes into this PR or open a PR for the update-controller-tools-and-revisit-multi-lines-comment branch?

@thbkrkr
Copy link
Contributor

thbkrkr commented Mar 8, 2024

@justenstall You could pull my changes but as you prefer.

… update-controller-tools

Signed-off-by: Justen Stall <justenstall@gmail.com>
@justenstall
Copy link
Contributor Author

@thbkrkr Pulled in your changes and resolved the merge conflicts with master. Tests are passing and it should be good to go.

cc @tenstad

@thbkrkr

This comment was marked as resolved.

@justenstall
Copy link
Contributor Author

@thbkrkr That logic makes sense to me, I'll add the change to this PR.

Signed-off-by: Justen Stall <justenstall@gmail.com>
@zirain
Copy link
Contributor

zirain commented Mar 12, 2024

when will this merged?

test/result.md Outdated Show resolved Hide resolved
@thbkrkr thbkrkr merged commit 15438a1 into elastic:master Mar 12, 2024
3 checks passed
@thbkrkr
Copy link
Contributor

thbkrkr commented Mar 12, 2024

Thank you for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade controller-tools to latest
5 participants