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

Fixed 'transport_name' option resolver check in QueueInteropTransport #82

Merged
merged 1 commit into from
Aug 11, 2019

Conversation

ekkinox
Copy link
Contributor

@ekkinox ekkinox commented Jul 29, 2019

Fixed 'transport_name' option resolving.

See related issue: #81

@Steveb-p
Copy link

It should work without those additional declared transport_name options in all other tests, so they didn't really need to be specified (and I don't feel that this new test adds value, other tests without those options would check that it still works implicitly), but overall 👍

@ekkinox
Copy link
Contributor Author

ekkinox commented Jul 29, 2019

It should work without those additional declared transport_name options in all other tests, so they didn't really need to be specified (and I don't feel that this new test adds value, other tests without those options would check that it still works implicitly), but overall 👍

If you check symfony/framework-bundle@5205108, 'transport_name' will be added automatically each time from 4.3.3, therefore I updated tests reflecting reality

agree on the new test, maybe a bit overkill just to show that it does not fail without 'transport_name' provided :)

@Steveb-p
Copy link

Steveb-p commented Jul 29, 2019

I you check symfony/framework-bundle@5205108, 'transport_name' will be added automatically each time from 4.3.3, therefore I updated tests reflecting reality

On second thought yes, you're right, Symfony will be adding it so it should be checked that it properly "works" when passed 😋

@chq81
Copy link

chq81 commented Aug 6, 2019

@sroze Any idea when you are able to merge it? I am desperately waiting for the fix :)

@Steveb-p
Copy link

Steveb-p commented Aug 6, 2019

@sroze Any idea when you are able to merge it? I am desperately waiting for the fix :)

@chq81 You can use composer's ability to overwrite package repositories to use your own fork with patch applied until this one is merged.

https://getcomposer.org/doc/05-repositories.md#loading-a-package-from-a-vcs-repository

There are a few use cases for this. The most common one is maintaining your own fork of a third party library. If you are using a certain library for your project and you decide to change something in the library, you will want your project to use the patched version. If the library is on GitHub (this is the case most of the time), you can simply fork it there and push your changes to your fork. After that you update the project's composer.json. All you have to do is add your fork as a repository and update the version constraint to point to your custom branch. In composer.json, you should prefix your custom branch name with "dev-". For version constraint naming conventions see Libraries for more information.

So, in your case it might be:

{
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/chq81/messenger-enqueue-transport"
        }
    ],
    "require": {
        "sroze/messenger-enqueue-transport": "^0.3.2 | dev->>BRANCH-NAME<<"
    }
}

Adding ^0.3.2 will allow you to update to a newer version once it's available (by using composer update sroze/messenger-enqueue-transport), if you have prefer-stable:true in composer.json config (https://getcomposer.org/doc/04-schema.md#prefer-stable)

@mitelg
Copy link

mitelg commented Aug 8, 2019

I'm also interested in getting this fixed 👍 Currently this prevents us from updating to Symfony 4.3.3 in https://github.com/shopware/platform

@chq81
Copy link

chq81 commented Aug 8, 2019

@Steveb-p Thank you very much for the detailed instructions.

@ekkinox
Copy link
Contributor Author

ekkinox commented Aug 8, 2019

I'm also interested in getting this fixed 👍 Currently this prevents us from updating to Symfony 4.3.3 in https://github.com/shopware/platform

yeah, on our side we froze to 4.3.2 for symfony/framework-bundle waiting for this to be merged

Copy link
Collaborator

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

I've tested this on a project.

Thank you

@sroze
Copy link
Owner

sroze commented Aug 11, 2019

Thank you @ekkinox!

@sroze sroze merged commit a707be9 into sroze:master Aug 11, 2019
@ekkinox ekkinox deleted the fix/interop-transport-name branch August 12, 2019 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants