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

fix: update to mergo 1.0.0 #377

Closed
wants to merge 1 commit into from
Closed

Conversation

caarlos0
Copy link

No description provided.

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
@nibbleshift
Copy link

nibbleshift commented Oct 11, 2023

@mattfarina Can we get this merged? This fixes a larger issue rather than just bumping up the version of mergo 1.0.0. Mergo switched their repo url from a gh url to a vanity url. See here: darccio/mergo#245

@andig
Copy link

andig commented Nov 8, 2023

@mattfarina please merge. The current experience is rather painful, even when that's an upstream issue:

go get -u

go: github.com/imdario/mergo@v1.0.0: parsing go.mod:
	module declares its path as: dario.cat/mergo
	        but was required as: github.com/imdario/mergo
	trying github.com/imdario/mergo@v0.3.16

@andig
Copy link

andig commented Nov 8, 2023

Note that readme also has this:

Sprig leverages mergo to handle merges. In its v0.3.9 release, there was a behavior change that impacts merging template functions in sprig. It is currently recommended to use v0.3.10 or later of that package. Using v0.3.9 will cause sprig tests to fail.

Not sure if upgrading to post 0.3.11 would break anything? Is fixing this a prerequisite?

@tantinhte
Copy link

please merge this PR

@stan-stately
Copy link

please merge this. its extremely annoying and its breaking random tooling such as renovate

@phuslu
Copy link
Contributor

phuslu commented Jan 13, 2024

I suggest decoupling from mergo to permanently solve this accident. @ALL

@andig
Copy link

andig commented Feb 3, 2024

Using v0.3.9 will cause sprig tests to fail.

@mattfarina I've re-run the tests on mergo 1.0- all fine:

sprig mergo
❯ grb master
Current branch mergo is up to date.

sprig mergo
❯ got ./...
ok  	github.com/Masterminds/sprig/v3	4.329s

@andig
Copy link

andig commented Feb 3, 2024

@caarlos0 would you mind updating the README, too?

@oliverpool
Copy link

For anyone impacted, add the following line at the bottom of go.mod in the meantime:

replace github.com/imdario/mergo => dario.cat/mergo v1.0.0

@caarlos0
Copy link
Author

caarlos0 commented Mar 8, 2024

For anyone impacted, add the following line at the bottom of go.mod in the meantime:

replace github.com/imdario/mergo => dario.cat/mergo v1.0.0

FWIW this doesn't actually work if your users do go install your-tool

@caarlos0
Copy link
Author

caarlos0 commented Mar 8, 2024

@caarlos0 would you mind updating the README, too?

which part?

@andig
Copy link

andig commented Mar 8, 2024

I think this needs be removed if no longer an issue:

IMPORTANT NOTES
Sprig leverages mergo to handle merges. In its v0.3.9 release, there was a behavior change that impacts merging template functions in sprig. It is currently recommended to use v0.3.10 or later of that package. Using v0.3.9 will cause sprig tests to fail.

@oliver-anz
Copy link

Bump, this would be very useful thanks

@ripienaar
Copy link

A fork is being considered, interested community members should consider adding their efforts there #396

@mattfarina
Copy link
Member

Thanks for pointing this out. Since I'm planning on a release, I created #406 to update to the 1.0.1 release of mergo and to handle the conflicts.

@ripienaar
Copy link

Thanks for pointing this out. Since I'm planning on a release, I created #406 to update to the 1.0.1 release of mergo and to handle the conflicts.

excellent news that a release is coming, would you be in a position to make some changes like remove the deprecated crypto algo functions by any chance?

Would really like to see sprig continue since the available forks have not really panned out - and forks aren't in anyones interest

@mattfarina
Copy link
Member

#406 handled the mergo change.

@ripienaar crypto functions can be deprecated but not removed without making a major API change. Consumers of this library enable those crypto functions to be used so I'm hesitant to remove them. This is why adding something new is always something we are hesitant of. You need to keep those APIs around for a long time.

Imagine if tools like Helm had template functions just vanish.

@mattfarina
Copy link
Member

Closing this PR as the intended change has already landed.

@mattfarina mattfarina closed this Aug 28, 2024
@ripienaar
Copy link

The ones I am talking about @mattfarina are the ones Go - and industry at large - have stated are unsafe, shouldn't be used etc

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.

10 participants