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

Remove upgrade steps for old, unsupported releases #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hvelarde
Copy link
Member

Version 2.2 is now 3-years old and we are not going to maintain it.

@hvelarde hvelarde self-assigned this May 30, 2018
@hvelarde hvelarde force-pushed the hvelarde-cleanup branch from 5691f5e to d940ad8 Compare May 30, 2018 20:12
Copy link
Member

@idgserpro idgserpro left a comment

Choose a reason for hiding this comment

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

I'm not a huge fan or removing old upgradeSteps and not creating a major version (like 3.x). I know we have:

 2.13b4 (unreleased)
.. Warning::
    Upgrading from versions below 2.3 is no longer supported.
    You must upgrade at least to version 2.3 before upgrading to this release.

But suppose I'm in 2.x, then upgrade to 2.13 final when it's released: this is a normal thought of upgrading, but since I didn't upgrade to 2.13b3 specifically now I have a broken installation.

@hvelarde
Copy link
Member Author

hvelarde commented Jun 6, 2018

@idgserpro I'm not a huge fan of maintaining old code in add-ons because it complicates our lives.

I have no problem bumping a major version if you feel more comfortable with that.

@hvelarde hvelarde force-pushed the hvelarde-cleanup branch from d940ad8 to daf44c1 Compare June 6, 2018 13:06
@@ -3,8 +3,8 @@ Changelog

There's a frood who really knows where his towel is.

2.13b4 (unreleased)
^^^^^^^^^^^^^^^^^^^
3.0b1 (unreleased)
Copy link
Member Author

Choose a reason for hiding this comment

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

@idgserpro done! do you mind if I remove now upgrade steps older than 2 years?

Copy link
Member

Choose a reason for hiding this comment

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

Since it's a 3.x release, I see no problem with that. But you may need to create a 2.x branch and be careful about upgradeSteps numbering like we had in a (portuguese) discussion in plonegovbr/brasil.gov.portal#471 (review)

That's why I prefer not to remove old upgradeSteps. It gives that feeling of removing and "cleaning" the package but creates another problems.

I believe our both approaches aren't wrong, it's just a matter of preference of which problems you want to live with.

@idgserpro
Copy link
Member

I'm not a huge fan of maintaining old code in add-ons because it complicates our lives.

I know that. See, I'm not against removing it, specially because you're the original maintainers of this project, and it's up to you to decide how to use your resources.

But try to imagine the scenario I pointed out in #162 (review): Imagine it's you upgrading from 2.x to 2.13 (final), everything is broken and after some researching you get to know that you need to upgrade first to 2.13b3 before upgrading to 2.13... such a MINOR change (semver nomenclature) shouldn't break like that.

But if it's a MAJOR change, at least some breakage is expected and if someone didn't read the changelog before upgrading from 2.x to 3.x, I'm sorry, it's their fault.

@hvelarde
Copy link
Member Author

hvelarde commented Jun 6, 2018

no, I'm not going to maintain any 2.x branch after this bump as it makes no sense.

I can create it, just because is the right thing to do, but I'm not going to touch it at all.

hvelarde added 2 commits June 6, 2018 15:34
Version 2.2 is now 3-years old and we are not going to maintain it.
@hvelarde hvelarde force-pushed the hvelarde-cleanup branch from daf44c1 to 6e905b1 Compare June 6, 2018 18:35
@hvelarde
Copy link
Member Author

@idgserpro do you agree? please approve.

@idgserpro
Copy link
Member

So, with a branch 2.x, if needed, a new upgradeStep will be 3011? 9 upgradeSteps at most since the 3.x package will begin with 3020?

@hvelarde
Copy link
Member Author

I think add-ons are different from projects like IDG: as mentioned in other place, we don't want to maintain a 2.x branch because it just don't make sense.

we are bumping the major release just to inform that we're breaking things.

@idgserpro
Copy link
Member

Well, a 2.x branch can be created in the future if if needed, a commit before this one.

@hvelarde
Copy link
Member Author

can you approve now? ;)

@idgserpro idgserpro self-requested a review June 12, 2018 17:47
Copy link
Member

@idgserpro idgserpro left a comment

Choose a reason for hiding this comment

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

I approve the code, but still think it's a matter of preference removing this stuff or not as said in https://github.com/collective/sc.social.like/pull/162/files#r193405091.

@Rudd-O
Copy link
Contributor

Rudd-O commented Feb 4, 2022

It's high time this project was cleaned up a bit and bumped to a newer major version. It's fine if you want to keep old compat code to ease upgrade between major Plone versions... but the project as it exists today has several bugs that should be fixed.

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

Successfully merging this pull request may close these issues.

4 participants