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

Use yaml.v3 to drop yaml.v2 direct dependency #1283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mikelolasagasti
Copy link

@mikelolasagasti mikelolasagasti commented Aug 21, 2024

Use yaml.v3 to drop yaml.v2 direct dependency.

see https://github.com/go-yaml/yaml#compatibility

@github-actions github-actions bot added the stale label Dec 30, 2024
@electron0zero
Copy link
Member

electron0zero commented Dec 31, 2024

can you rebase, resolve conflicts, and add a meaningful PR description with "what?" and "why?" for this change.

@github-actions github-actions bot removed the stale label Dec 31, 2024
This update replaces `yaml.v2` with `yaml.v3`. While the change does not
directly impact blackbox_exporter functionality, `yaml.v3` provides
improved features, better YAML 1.2 compliance, and security
enhancements. Upgrading ensures the project stays aligned with best
practices and benefits from the latest module improvements.

Signed-off-by: Mikel Olasagasti Uranga <[email protected]>
@@ -30,7 +30,7 @@ import (
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/prometheus/common/expfmt"
"github.com/prometheus/common/promslog"
"gopkg.in/yaml.v2"
"gopkg.in/yaml.v3"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see @SuperQ's comment about blockers around upgrading to v3. go-yaml/yaml#665 (comment)

so I will hold and wait for @SuperQ's review on this before we merge this.

@electron0zero
Copy link
Member

electron0zero commented Jan 1, 2025

I would love to get this in and get the new features like comments, but looks like this is bit more involved then just updating the dependency.

There is a behaviour change, and I am afraid that might break people's config. If and when we do this, we will need to either verify that this is not breaking config for users, and if it's breaking, we will need to give advance notice.

also see:

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.

2 participants