-
Notifications
You must be signed in to change notification settings - Fork 435
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
feat(GPS): allow send attributes in Google PubSub message. #1349
feat(GPS): allow send attributes in Google PubSub message. #1349
Conversation
@makasim Can someone look at this PR please ? |
Hello @dgafka. |
as i saw you approve some requests and i don't have any responses, i ask you but i understand. |
ci pipeline does not trigger for this PR. I dont know why. I cannot merge PR without it being tested on CI succesfully. |
i see why the CI is not trigger. |
I don't understand how to run CI. |
Hi @makasim, can you run CI actions, please? |
I cannot, I would do it if it were possible |
I hesitate if it was a trigger message. |
CI has to be triggered on any MR by default without any additional actions from any side. There is one exception where a maintainer has to approve CI but there is none. |
Maybe try open a new PR from new branch, hope it helps |
@makasim i push a fix, can you run the CI please. |
@makasim i push a change, can you run the CI please. |
1 similar comment
@makasim i push a change, can you run the CI please. |
8de13b9
to
8495deb
Compare
@makasim I push a change, can you run the CI please. |
@makasim We found that the 404 error come from an update of aws/aws-sdk-php. |
7175191
to
d32e67e
Compare
d32e67e
to
21a36c9
Compare
@makasim I push a change, can you run the CI please. |
@makasim it's seem the image use to run CI not found the docker compose command. |
@makasim I push a change, can you run the CI please. |
@Steveb-p i try something can you run the CI ? |
@Steveb-p and @makasim, the CI fail on the SQS and SNS. |
@Steveb-p can you run the CI please ? |
Thanks @Steveb-p |
Let me review the code after work, just in case. |
@Steveb-p can you run the CI and take a new look on the code please ? |
9ce3f7c
to
10dea2a
Compare
@Steveb-p, I push the missing code style. |
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.
LGTM.
I will leave it for a while before merging, probably for a day.
Unless there are any comments from @makasim or anyone else.
Forgot the part to get the attributes on receive a message. |
bin/changelog
Outdated
#git add CHANGELOG.md && git commit -m "Release $1" -S && git push origin "$CURRENT_BRANCH" |
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.
uncomment please, I need it while releasing new tag.
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.
sorry
docker-compose.yml
Outdated
@@ -127,13 +127,13 @@ services: | |||
- '9090:9090' | |||
|
|||
localstack: | |||
image: 'localstack/localstack:0.8.10' | |||
image: 'localstack/localstack' |
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.
set a fixed version please
Few comments from me, Almost there. |
@p-pichet big thank you for your tireless work. Enqueue's CI is green and it not only provides the feature you need but unblocks a lot of other stuck MRs. Kudos to you, much appreciated! |
Thanks a lot. |
Allow to send PubSub attributes