Skip to content

Conversation

stevenbrookes
Copy link
Contributor

New SQS AddPermission operation, required to allow 3rd parties the permission to send messages to queues created using the createQueue endpoint.

I didn't seem to need to do anything after using the generate script - very cool. Let me know if I've missed anything.

As it looks like you are using a monorepo how would I go about using my fork of that in my code while the PR is reviewed and hopefully accepted.

Thanks

Steve

@GrahamCampbell
Copy link
Contributor

I didn't seem to need to do anything after using the generate script - very cool. Let me know if I've missed anything.

You need to write the tests.

@stevenbrookes
Copy link
Contributor Author

Test added. I seem to be failing the change log test. I added an entry, but perhaps not the right format? Any advice?

@jderusse
Copy link
Member

jderusse commented Oct 1, 2024

Test added. I seem to be failing the change log test. I added an entry, but perhaps not the right format? Any advice?

You can ignore the @dev suite (it should be fixed by #1771 )

The @stable just need an entry in the CHANGELOG file (you should add a ### Added section and a descript of your change)

@jderusse
Copy link
Member

jderusse commented Oct 3, 2024

The last failing test is about bumping the minor version.

Because this PR adds a new feature, the next version of the package will change.
You need to edit the src/Service/Sqs/composer.json file and set 2.2-dev in the branch-alias section

@stevenbrookes
Copy link
Contributor Author

Yay finally passed the tests. May I suggest adding the additional things that need to be done to the contributing documentation so aid future contributors? I look forward to this PR being accepted.

@jderusse jderusse marked this pull request as ready for review October 3, 2024 09:32
@jderusse jderusse merged commit 17ff869 into async-aws:master Oct 3, 2024
16 checks passed
@jderusse
Copy link
Member

jderusse commented Oct 3, 2024

thank you @stevenbrookes

Please yes, fill free to improve the documentation 🙏

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.

3 participants