-
Notifications
You must be signed in to change notification settings - Fork 436
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
GPS: revert the attributes and use the headers instead. #1355
GPS: revert the attributes and use the headers instead. #1355
Conversation
@makasim can you review it please ? |
@p-pichet the code you added, with explicit attributes, has just been released. Technically, now you're breaking the changes you just introduced. Backwards compatibility breaks are a big no-no for libraries. You have to explain it precisely why you want to go back, and what caused you to change your mind - links to resources, discussion that proves the issue, code samples, and so on. |
@Steveb-p, when I use it on my project, I don't have access to attributes in the consume message. I check on the InteropQueue and found that I make a mistake of global compatibility. Base on this comment :
The GPS attributes are simply headers. By using a custom property What can I propose to avoid breaking change is this :
What do you think ? |
@Steveb-p i push a commit to deprecate the attributes. If you have time to take a look. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Steveb-p i push a commit to deprecate the attributes.
If you have time to take a look.
The library is technically "not stable" being in 0.10 version series, so in theory it's fine to experiment.
A bit.
I appreciate your work done for the tests, but if you say that you need a particular feature and then it turns out it didn't work in your project, I will double check everything from now on.
I will look on it later on, but I will be busy in the next few days.
aea4844
to
c8c4993
Compare
@p-pichet Don't worry :) It's just that when you're providing software for someone else, suddenly changes you can make in your own project that you were able to do are forbidden :) If I can offer you a bit of advice - when working on a project and using open source libraries that you need to change to work, you can create your own forks and install them in place of the original. See https://getcomposer.org/doc/05-repositories.md#repository . For example, you can fork both enqueue-dev and sroze adapter, and do the changes that you need to. They can both be installed using VCS... {
"repositories": [
{
"type": "vcs",
"url": "[email protected]:my-fork/enqueue-dev.git"
}
]
} ...or even local paths:
And you can install branches pretending to be stable releases:
(at least I think that's the syntax) This is also one way of getting the features you need in your project, while waiting for your PRs to be approved. |
I found that i was go in wrong direction.
The GPS attributes are the Interop Message headers.
I revert to use the
headers
and remove theattributes