search

sroze / messenger-enqueue-transport

Uses Enqueue with Symfony's Messenger component.
MIT License
191 stars 55 forks source link

Update Travis matrix to have a global overview and to allow testing on beta version. #37

Closed shulard closed 5 years ago

shulard commented 5 years ago

As we discussed in #36, this PR goal is to have a complete Travis matrix to :

I've also updated the composer.json file which is currently in a "dev" minimum stability requirement.

Regarding the test results, the current master branch is compatible with Symfony/Messenger 4.2 but not with the 4.1 version anymore…

What do you think about these details ?

shulard commented 5 years ago

The Travis build is failing because the master branch is not compatible with the stable package versions. We got this issue in the tests :

......PHP Fatal error:  Interface 'Symfony\Component\Messenger\Stamp\StampInterface' not found in /home/travis/build/php-enqueue/messenger-adapter/EnvelopeItem/TransportConfiguration.php on line 23

I'm not sure that this PR must update the composer stability level but it help viewing the current state of the master branch.

Do you plan to maintain compatibility with Messenger 4.1 at the beginning ? If so, the new build details used in that PR can help checking for errors.

drgomesp commented 5 years ago

To me, it makes sense that master doesn't provide support for 4.1 anymore – one has to be chosen as the target release for any given branch. In fact, master should be compatible with dev-master of Symfony.

Dropping support of the 4.1 API on master seems ok to me. Thoughts, @makasim ?

makasim commented 5 years ago

Dropping support of the 4.1 API on master seems ok to me. Thoughts, @makasim ?

yeap, it's ok for me too.

drgomesp commented 5 years ago

@shulard can you revert the composer changes so we can make this pass with at least 4.2?

shulard commented 5 years ago

Yes and no, the changes that I've made on the .travis-ci.yml file are not compatible with the current composer.json state.

I force to use the stable versions by default then execute a build on beta and dev versions. Since we are requiring to use dev in master for the moment I must not bypass the composer stability when not necessary. I try to do those changes ASAP.

shulard commented 5 years ago

Hello guys,

Since I received the Symfony 4.2 release email in the morning and I had some time to review that PR, I updated the composer.json to match the new 4.2 stable requirements.

I also updated my .travis-ci.yml changes to never bypass the composer.json default value in the important build (the one that are not allowed to fail :smile:).

Please tell me if you think I need to update one of those two files !

makasim commented 5 years ago

LGTM

drgomesp commented 5 years ago

Thanks @shulard. I'll merge this and then we can tag, and change master to 4.2@dev again. Makes sense, @makasim ?

shulard commented 5 years ago

@drgomesp :+1:

For me moving back to 4.2@dev is not a nice solution. This library must be compatible with the current state of the ecosystem. Moving to dev was the solution because we rely on specific interfaces updated in 4.2 and 4.2 was not released. master must be released ready most of the time (or when there is a choosen BC Break regarding versions).

makasim commented 5 years ago

The messenger is still experimental so it is likely that some interface might be changed. I guess it could happen only in 4.3 version, not 4.2 releases. But I don't know that for sure.

I think that we should test against stable version as well as latest dev.

shulard commented 5 years ago

Totally agree, and it's the main goal of the updated travis configuration in that PR. With these changes we are testing against beta and dev version of the dependencies. For the moment those build are allowed to fail maybe we can make them required if they are important...