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

docs: add about removal of deprecated functionality #7021

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Dec 25, 2022

Description
See #7000 (comment)

  • update backward_compatibility_notes.rst

At least Autoloader::loadLegacy() was removed in 4.1.0.
See https://codeigniter4.github.io/CodeIgniter4/installation/upgrade_410.html#breaking-changes

Checklist:

  • Securely signed commits
  • [] Component(s) with PHPDoc blocks, only if necessary or adds value
  • [] Unit testing, with >80% coverage
  • User guide updated
  • [] Conforms to style guide

@kenjis kenjis added the documentation Pull requests for documentation only label Dec 25, 2022
@kenjis kenjis mentioned this pull request Dec 25, 2022
5 tasks
Copy link
Member

@michalsn michalsn left a comment

Choose a reason for hiding this comment

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

Thanks, @kenjis!

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

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

Let's make sure @lonnieezell signs off on this. If we shift Semver such that our minor releases are the equivalent of their major releases then I think that's circumventing the issue of major release cycle timing. That said, I still believe there are things we need to "break to fix", and the Config\App deprecations are a good example of this (rather than removing deprecated items just for the sake of it).

@kenjis kenjis requested a review from lonnieezell December 26, 2022 21:25
@kenjis
Copy link
Member Author

kenjis commented Dec 26, 2022

I have sent the PR #7000 to remove the Session items in App\Config because of a bug? or flaw? that prevents users from using database due to the dependency.

This PR does not mean removing all deprecated items aggresively.

However, I would like to remove all dependencies on the Config\App that are no longer needed due to the new Config files, if possible. Because they requires a lot of work to maintain the code for backward compatibility.

@kenjis
Copy link
Member Author

kenjis commented Jan 4, 2023

@lonnieezell Your opinion?

@lonnieezell
Copy link
Member

Here's the deal - this addition tells me, as a developer using the product, that there is no guarantee that updating for a security fix isn't going to break my application. That kind of a headache would make me look for a more stable, honestly, of which there are several.

Don't get me wrong, I understand sometimes things have to break to fix but I think we should strive to do that as little as possible. "Switching Semver" means abandoning Semver. There are whole tool chains that expect Semver to actually mean something and we can't change the meaning of it for us and have it still work well with the tools that developers are using and counting on.

What is "mature code"? How can we say that after 4 major versions over the 2 decades)+/-) CI has existed, and the 41 releases of v4 alone that it's not "mature"? Not a fan of that phrasing.

As for removing deprecations, if it changes the public API Semver says that's a major release, if we care. I'm not convinced that moving items out to a new config file is quite the same thing as changing a public API, but it's probably close. So, I don't necessarily mind the change that brought this PR up.

I do have an issue saying we might break things at any time, especially patch releases. We have made a contract with the developers in the past with the text right above this change (which contradicts what this change is saying):

Only major releases (such as 4.0, 5.0 etc.) are allowed to break backward compatibility.
Minor releases (such as 4.2, 4.3 etc.) may introduce new features, but must do so without breaking the existing API.

To change that outside of a major release feels like a huge break in confidence.

@kenjis kenjis marked this pull request as draft January 17, 2023 01:11
@kenjis
Copy link
Member Author

kenjis commented Jan 24, 2023

I sent a new PR #7173

On the matter of patch releases that might break the existing apps.
It should be banned if possible, but it has actually happened many times in the past.
Since it seems impossible to raise a major version with bug (security) fix release,
and it may happen in the future, so I think it is difficult to assure that it will not happen.

I think CI4 code is not mature. It is rewrite in many parts, so it is new.
CI3 code is mature. It has been used in long time and It will not be changed no more.
However, if the term mature is inappropriate, you all may change it.
My intention was to describe that a bug fix can cause a breaking change in a patch release.

@kenjis kenjis closed this Jan 24, 2023
@kenjis kenjis deleted the fix-docs-backward_compatibility_notes branch January 24, 2023 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests for documentation only
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants