Beta release v2
dereuromark opened this issue · 18 comments
We would like to make a new beta release.
It will contain a few (internal) BC breaks due to added types for param and return.
Overall this will help to stabilize the API, however, since it becomes now more visible what actual content goes in and out (and adjust the docblocks accordingly).
So any further releases (beta, RC, stable) should then be able to build on this improved API and further breaks should then hopefully be not necessary anymore.
Required Tasks:
- PHP8.1+ fixes and PHP 8.2 early compatibility check - #1811
- Fix up public and internal API further regarding types (less type per method the better) - #1656
- Make sure all generated code is also strictly typed as good as possible.
- Analyze BC impact for extensions and extending code. In certain cases we might have to revert added types maybe?
- Cleanup from e.g. #1833 left overs (CS, ...)
Nice to have (if someone has time the next days):
If all is done, we will have one QA run and then release/tag a new beta version.
Any help on the above points are welcome, also please check the current master with your project(s) then to see if something might be missing or not covered by tests.
Open issues found during stricter typing:
- moveToBottom() => fix to be sync with moveToTop() with clear return type ($this probably)
Currently it is a dirty mix of the fluent interface and some prev value as return value. - Fluent interface (chainable) for ObjectBuilder void methods
public function init$relCol(\$overrideExisting = true)=> void or $this ? and other methods here
=> We can keep void for now - Line Propel/Runtime/Collection/OnDemandCollection.php
116 Return type (Propel\Runtime\Collection\OnDemandIterator) of method Propel\Runtime\Collection\OnDemandCollection::getIterator() should be compatible with return type (Propel\Runtime\Collection\CollectionIterator) of method
Propel\Runtime\Collection\Collection::getIterator() => #1889
Anyone able to help out with some of the open tasks?
So we can finalize this next week?
@oojacoboo Anything from your side?
You seem to be the one most active these days
Before we close that milestone and release a new beta?
@dereuromark I'll have a look if I can get a moment. I did get a rather large codebase using the master branch last night. I ran into a few method signature compatibility issues and a broken path for a Propel extension we maintain (soft-delete for Propel2). The template path was due to a relative path issue - resolved with a realpath usage in the extension. Other than that, all tests are currently passing. It's possible there are some other remaining issues that'll come up, but so far everything seems fine. It's likely people will have to update their code for the latest changes though. Of note, all the lifecycle method signatures have changed, so any use of them will have to be updated.
Thats good to hear.
Yes, we will have to outline the major changes in the release notes.
We are now officially closing any further changes to the master branch if there are no objections or regressions/issues detected.
We start QA phase and then will release as planned.
We have failing Psalm right now
https://github.com/propelorm/Propel2/runs/6687142438?check_suite_focus=true
Is anyone able to help out here?
Is there a PR for any of this?
No, someone would have to create one to make the build (master) green.
@oojacoboo did u have a chance to check?
@dereuromark started diving in. I cannot tell how to get the test environment properly setup, locally. This lib needs docs on running testing tools locally.
The GitHub CI results do not match my local results. PHPUnit tests fail b/c of missing RDBMS servers. GitHub CI results are scanning the vendor dir, and the first error in those results above is incorrect, unless vendors weren't properly installed.
I've created a PR, however, and people can bang on it until we get something working. Feel free to commit there.
@oojacoboo There is documentation for running the tests at http://propelorm.org/documentation/cookbook/working-with-test-suite.html
However, I think the documentation is outdated, so there is still some guesswork/re-engeneering required. For example, it says that you can change DB username and password through environment variables, but I am pretty sure that at least mysql only works with username 'root' and password as empty string. I agree that this is a problem, both the outdated documentation and having to run the tests as root.
If you are still trying to get this to work, maybe open a new issue where we can look at it together?
@mringler thanks for the doc reference. I feel like the test dir should have a README. I get the desire to put docs on a website for readability. But searching around on a website really isn't ideal for OSS development. It's good for lib users. Test docs aren't for lib users, they're for devs.
@dereuromark all the tests are passing now in #1886
Cool, lets try to wrap this up this week so we can finally tag this by the end of the week maybe?
OK, we would start tagging tonight or tomorrow
Any objections?
@pablogd @oojacoboo @mringler
Are you OK with the release now?
We would tag it today after we tested it locally and it seems so far fine also for usage in our framework and project.
I've been able to test this on a rather large app and don't see any issues. It's worth noting that there are definitely BC breaks with regards to all lifecycle methods and base "action" methods (save, delete, etc.), so release notes should make that clear. But with the versioning I don't see that as an issue at all.