feat(bindings): add SNS AWS bindings #108
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you dpwdec for your first pull request to AsyncAPI.NET repository. Please check out our contributors guide.
dpwdec
changed the title
Co-authored-by: UlrikSandberg <[email protected]>
|
Its starting to look really good. Any feedback on the implementation side, to make things a bit easier? |
@VisualBean It has been pretty good so far. Much smoother than most other open source projects I’ve contributed to. I think you’ve done a good job keeping it flexible but stream lined. My main thing would be that the project just needs more documentation for these things. Some of the helper functions introduced for defining bindings I just of figured out through trial and error as they don’t have any explanatory doc strings so I very much feel like “am I using this as intended” a lot of the time. I guess some explanation on the architecture would be pretty helpful as well. Why is the project structured/implemented in this way? Maybe ADRs would be good format for doing this. I suppose it feels a little verbose to have to define reading AND writing as well as the model. But I think I can understand why you’ve done it this way given the constraints of AsyncAPI. This is kind of going towards a feature request. But for properties where the input / output match exactly with the model apart from casing it seems like just being able to define that relationship directly without explicit map or writer would be a very nice addition. It’s a hard one because clearly there’s lots of cases where you might also want that fine grained control over different I/O but I’m finding for most of the stuff I’m working with 80% is pretty straightforward and then there’s a 20% that’s a bit more involved and requires the custom flexibility. Will be interesting to see when I try creating a custom binding for my own project as well. |
|
I’m more than happy to do a docs PR after this though to document some of what I’ve learned so that other people with no exposure to the project might be able to get involved more easily |
VisualBean
force-pushed
the
main
branch
2 times, most recently
from
ad41486 to
5952b24
Compare
BREAKING CHANGE: Bindings have been moved to a separate project
VisualBean
changed the title
VisualBean
changed the title
|
@VisualBean This one should also be good to go now as well, integrated the |
| public Protocol Protocol { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// Where are messages being delivered to? |
There was a problem hiding this comment.
it seems odd that these are questions. 😅
There was a problem hiding this comment.
@VisualBean updated! 🙇♂️ (I will reflect the change in binding specification as well)
|
Great work @dpwdec. Ive merged and pushed the official v4. Thanks a lot for your contributions and commitment |
|
@VisualBean And thanks for all your help and support with getting it reviewed and completed! 🙏 |
About the PR
Add SNS bindings
Changelog
Related Issues
None