vfsStream icon indicating copy to clipboard operation
vfsStream copied to clipboard

Refactor and rename

Open mikey179 opened this issue 5 years ago • 8 comments

As discussed in #213 I took the opportunity to rename some classes. Also, in https://github.com/bovigo/vfsStream/pull/220#pullrequestreview-362513234 I mentioned I believe there's opportunity to restructure code. This is my attempt at that. It probably has some rough edges and I'm not sure it's a good idea to merge this big one. On the other hand, this is a big opportunity, as we will have a new major version anyway, and I managed to reduce the amount of code while all features are kept. Please have a look and let me know what you think.

mikey179 avatar Feb 25 '20 15:02 mikey179

In my last commit I updated the org layer to load the new classes. However, I'm a bit unsure how to continue:

  • Remove all classes/interfaces from org which don't have a match any more? These are mostly ones which people typically shouldn't have used, but you never know... but it would break the current promise that an update can be done seamlessly.
  • Does the org layer actually still make sense with the renaming?
  • Or should it be two steps - a release 2.0 with the namespace change only, quickly followed by a 3.0 with the rename in place?

I'd like to hear your thoughts on this.

mikey179 avatar Feb 28 '20 09:02 mikey179

  • Does the org layer actually still make sense with the renaming?
  • Or should it be two steps - a release 2.0 with the namespace change only, quickly followed by a 3.0 with the rename in place?

I was a little curious about this, too. The rename does seem like it makes the old org layer somewhat pointless if all the files don't match up anymore. If the goal is to maintain an easy transition, putting this into 3.0 does sound like a better idea.

If we go that route, then we can update the org files that don't have a match anymore to include "this file will be removed in 3.0" to the deprecation message.

Then for any development, we'll just have to create a 2.x and remember to branch off of that. Merging any 2.x changes into 3.x could be fun...

bizurkur avatar Feb 28 '20 12:02 bizurkur

Merging any 2.x changes into 3.x could be fun...

Yes, that's one of the thoughts I had - we might create a maintenance problem for us. So in case we opt for two major releases (not necessarily at the same time) we should limit the support time span for 2.0 - I'd even go to say that it should receive bug fixes only.

mikey179 avatar Feb 28 '20 14:02 mikey179

Over the weekend I thought a bit about this and came up with the following idea:

  1. Merge the namespace change into 1.x.
  2. Rename the classes that will receive a new name, adjust the org namespace accordingly to reference the new names, and to declare the classes/interfaces which will be removed because they are not present any more after the refactoring.
  3. Release this as 1.7.0.
  4. Remove the org layer with the old namespace from master.
  5. Merge this one into master, and continue the work with the other issues and open PRs.
  6. Once we deem master ready for release it becomes 2.0.0.

This has some advantages:

  • People can upgrade to 1.7 without any problems, and start to adjust their code in preparation for 2.0. But they don't have to do this immediately with the upgrade.
  • Once people did the changes that become possible with 1.7 an upgrade to 2.0 should not require any major work.
  • We decouple our further work on 2.0 from the namespace change.
  • There's only one major release on the time plan, not two as with my initial thoughts.

What do you think? Have I overlooked anything that might pose a problem?

mikey179 avatar Mar 02 '20 09:03 mikey179

That sounds way better and should have far fewer complications 👍

bizurkur avatar Mar 02 '20 12:03 bizurkur

What is the status of this pr? I would like to start working on php8 compatibility. But this large move of files makes me feel that I have to wait for this PR.

I reviewed the changes, I think this one is good to go...

jaapio avatar Jul 03 '20 12:07 jaapio

Agreed. What else is left in this PR?

I'd like to get a migration path from 1.7.0 to 2.0.0 so we can finally EOL the 1.x series.

allejo avatar Jul 03 '20 22:07 allejo

If I remember correctly, within the plan I outlined above this should now be at point 3. Unfortunately for this PR I got pulled into a large project mid March and are lacking any time to continue working on this. I'm not sure if this one could be merged as is or if there were some other changes that should be done, something in the back of my mind tells me I wanted to look at something but I can't remember what it was. Probably it's ok to check with #227 if something needs to be done here before it should be merged so the change is consistent in 1.x and master.

mikey179 avatar Jul 06 '20 11:07 mikey179