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

Make upgrading to 3.0 easy for us #514

Open
johnnyshields opened this issue Jan 5, 2022 · 11 comments
Open

Make upgrading to 3.0 easy for us #514

johnnyshields opened this issue Jan 5, 2022 · 11 comments
Assignees

Comments

@johnnyshields
Copy link
Contributor

johnnyshields commented Jan 5, 2022

In order to make upgrading to version 3.0 easy, can you do something like this on the 2.x series:

      def write_buffer(io = ::StringIO.new(''), legacy_encrypter = nil, encrypter: nil)
         encrypter = legacy_encrypter if legacy_encrypter
         ...

And do that everywhere there is an arg change. That would be lovely, thanks!


EDIT: originally I also suggested to warn("DEPRECATED: ... should be a keyword arg") but I've removed this based on comments.

@hainesr
Copy link
Member

hainesr commented Jan 9, 2022

I'm happy to consider this, but I caused people issues when I added things to STDERR recently (see #497). So I'd be interested in what people like @jdleesmiller, @ShockwaveNN, @tisba and @Sega-Zero think before I do this. Part of the issue is that rubyzip can be a very deep dependency that people don't (in some cases) realise is there until it does something like this.

Also, frankly, it's taking long enough to polish off 3.0 as it is - this isn't my day job! - and this would surely add more time as it is, what with all the extra testing, etc required.

Anyway, as I say, happy to look at doing this so long as it's not going to cause people unexpected problems elsewhere.

@Sega-Zero
Copy link

Well, in my case, I fixed my build scripts. But this may certainly be an unexpected surprise for someone else. This kind of information would be much more helpful in the migration section of the documentation.
For those who really are going to upgrade their sources, it will be much more effective rather than a warnings that may be easily skipped by a developer for some reason. IMHO.

@ShockwaveNN
Copy link
Contributor

@hainesr I don't like the previous variant because it shows a warning on each call of rubyzip (as far as I remember, was a quite ago)

But it banner will be shown only on the call of deprecated code (which should be replaced in my code\some library code) I think it's OK

@johnnyshields
Copy link
Contributor Author

You're right--let's forget about warn(...) and instead just do the method interfaces on 2.x branch. I've updated the original post.

@tisba
Copy link

tisba commented Jan 10, 2022

I can't read the original suggestion (was edited).

I think it is not too bad to use Kernel.warn with a proper category set. This can be managed by the warning gem or rubyzip could allow to disable those warnings directly.

Personally, I'd just add a breaking change with a major bump and be done with it 🤷

@hainesr
Copy link
Member

hainesr commented Jan 10, 2022

Thanks all for the comments!

Personally, I'd just add a breaking change with a major bump and be done with it 🤷

In an ideal world I would just do this 😆 But there are an intimidating number of projects that use rubyzip (GitHub alone reckons over 1.7M) so I only need a small percentage of them to get caught out to ruin my week after the release 😟

I'd like to find some middle ground to make it as smooth as possible for people to update, so I'll look at a combination of things suggested in this thread. I'll leave this issue open while I do this.

@tisba
Copy link

tisba commented Jan 10, 2022

Okay if people without any experience "just" use it (meaning just installing whatever version of rubyzip is latest), then I do see your point. 🤔

What about printing the warning at least once? There are plenty of options:

  • if somebody doesn't care about any warning, they can run with -W0
  • they can use the warning gem to filter it out
  • you could remember that the warning has been printed once and omit it for subsequent calls
  • printing a warning could be disabled by something like RUBYZIP_DISABLE_DEPRECATION_WARN=1 in the env

If your concern is though that even a small percentage might run into this, because they don't have a process for updating their dependencies (for example), I believe none of these options would work. The only option would be to support the old API indefinitely.

@ShockwaveNN
Copy link
Contributor

you could remember that the warning has been printed once and omit it for subsequent calls

I think it's an overcomplication of this problem, to save state of warning message

printing a warning could be disabled by something like RUBYZIP_DISABLE_DEPRECATION_WARN=1 in the env

My first thought was to add rubyzip option hide_warnings, but I think a lot of people use rubyzip as inderenct dependency and you cannot change call options of another gem easily, without forking it

But ENV may be a little non-elegant solution but will work in those scenarios

@hainesr hainesr self-assigned this Jan 10, 2022
@hainesr
Copy link
Member

hainesr commented Jan 10, 2022

The only option would be to support the old API indefinitely.

Oh, the old API is already gone from trunk so no danger of that 🤣

My first thought was to add rubyzip option hide_warnings, but I think a lot of people use rubyzip as inderenct dependency and you cannot change call options of another gem easily, without forking it

Yes, I thought about a rubyzip option too, but that is a good point about indirect dependencies. I think the ENV solution is probably a good idea, especially as it's a temporary thing for the end of the 2.x branch anyway.

@sandstrom
Copy link

Perhaps this can be closed?

8cec949

@ccutrer
Copy link

ccutrer commented Aug 22, 2023

I would really appreciate a 2.x release that's forward compatible with 3.0 by supporting the new kwargs in the options hashes that already exist. My project, in addition to using RubyZip itself, depends on 5 other gems that also use RubyZip (and have proper requirements not allowing RubyZip 3.0). As it stands right now, any given gem cannot support both RubyZip 2.3.2 and 3.0 at the same time without significant gymnastics. Because of this, I cannot update any one of my dependencies that depend on RubyZip 3.0 until all of them do. If a 2.x is released that supports the new kwargs in addition to the old way (even as part of the options hash, and not as proper kwargs), then intermediate gems would be able to support RubyZip 2.x and 3.0 with the same code, and my end-project could update each one as it's available, and finally update its own usage to 3.0 when all of my dependencies have updated.

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

No branches or pull requests

7 participants