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

Noisy banner shown after 2.3.1 upgrade #497

Closed
henrik opened this issue Jul 5, 2021 · 13 comments
Closed

Noisy banner shown after 2.3.1 upgrade #497

henrik opened this issue Jul 5, 2021 · 13 comments

Comments

@henrik
Copy link

henrik commented Jul 5, 2021

2.3.1 added a banner that is a bit noisy: https://my.diffend.io/gems/rubyzip/2.3.0/2.3.1

Perhaps this could be part of a changelog instead? I'm thinking any incompatible changes could follow semver and users could be relied upon to be careful about major-version upgrades. What do you think?

@ShockwaveNN
Copy link
Contributor

Agree banner on each run of my code even I don't using this code mention in banner is bit to much

Maybe this should be added to gemspec post_install_message?

@dmke
Copy link

dmke commented Jul 5, 2021

Since 2.3.1 is only "a dummy release" (see changelog), I guess the current workaround is a version constraint like this:

# 2.3.1 is a dummy release, and only brings a warning
gem "rubyzip", "~> 2.3", "!= 2.3.1"

The correct solution™ would be upgrading to v3, but unfortunately, that's not released yet...

@Sega-Zero
Copy link

That dummy release broke my builds today... Very useful for me (it's not)

@tisba
Copy link

tisba commented Jul 5, 2021

That dummy release broke my builds today... Very useful for me (it's not)

Can you confirm that this is because of the banner/warning?

@Sega-Zero
Copy link

yep.
 #1 2021-07-05 20-48-09
the string in the rectangle should have been the only data my scripts were expect

firebase_management_list plugin in fastlane instead gave me the whole useful warning stuff in the output.

All further script steps failed because of mailformed url.
Had to fix this in Gemfile. I could post an issue to the plugin repo, but I think it's not plugin's fault

@henrik
Copy link
Author

henrik commented Jul 5, 2021

A more actionable pattern I've seen is to introduce e.g. a 2.99 some time before a v3.0 release. 2.99 supports both old and new APIs, but shows deprecation warnings when you use the old ones. That lets you spot issues ahead of time and you can switch to the new API to get rid of the warnings :)

@tisba
Copy link

tisba commented Jul 5, 2021

@Sega-Zero Disclaimer: I don't know fastlane or that plugin.

It looks like the banner (which is annoying for sure) is using Warning#warn via Kernel#warn which prints to STDERR (can be tested via ruby -e "warn 'warning'; puts 'hello world'" 2>/dev/null). If your build step is sensitive to output on STDERR (which sounds a bit strange, TBH), you could use RUBYOPT="-W0" to disable warnings.

Again, I'd prefer if the warning is not printed just because you require the gem. That's a bit excessive 😀

@Sega-Zero
Copy link

@tisba I would agree of that, but why should I bother when using some gem that uses some gem that uses rubyzip, that rubyzip is about to upgrade to 3.0? Well, now i do now that :)

@hainesr
Copy link
Member

hainesr commented Jul 5, 2021

Apologies if this has caused people issues. The intention was to warn people that version 3 is really not compatible with version 2.x and previously we had people complain about not being warned about major version releases. We can't do the deprecated message as suggested by @henrik I don't think, because we're not deprecating, we're changing to named params. In any case this would still cause issues for people who rely on STDERR to be left unchanged. I thought warn was considered safe and standard usage...

Personally I'm comfortable with SemVer but it seems a lot of people hadn't heard of it when we released 2.0, which is why we did this this time.

So... I'm happy to yank this one if it's a real problem for people. I can can move to using post_install_message instead.

@tisba
Copy link

tisba commented Jul 5, 2021

So... I'm happy to yank this one if it's a real problem for people. I can can move to using post_install_message instead.

Please, please! 🙏 Don't yank this release. That will only cause more trouble. Just make it a patch release if you want to change the warning e.g. to a post install message.

@ShockwaveNN
Copy link
Contributor

I agree, yanking gems usually cause more problems, than helps to resolve

But releasing 2.3.2 with warning moved to post install is defenetly a good idea

@hainesr
Copy link
Member

hainesr commented Jul 5, 2021

Right, 2.3.2 released with post_install_message and no banner on STDERR.

Sorry for the noise folks. Lesson learned!

@hainesr hainesr closed this as completed Jul 5, 2021
@tisba
Copy link

tisba commented Jul 5, 2021

No problem at all, @hainesr! Thanks for your work!

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

No branches or pull requests

6 participants