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

Document the aws jslib's EventBridge support #1348

Merged
merged 2 commits into from
Oct 4, 2023
Merged

Document the aws jslib's EventBridge support #1348

merged 2 commits into from
Oct 4, 2023

Conversation

oleiade
Copy link
Member

@oleiade oleiade commented Sep 28, 2023

This Pull Request documents the newly introduced support for the AWS EventBridge service.

⚠️ this PR should be merged only after the jslib-aws's version 0.10 was released ⚠️

@oleiade oleiade self-assigned this Sep 28, 2023
@oleiade oleiade added the Area:extensions Issues about the extension ecosystem label Sep 28, 2023
@github-actions
Copy link
Contributor

There's a version of the docs published here:

https://mdr-ci.staging.k6.io/docs/refs/pull/1348/merge

It will be deleted automatically in 30 days.

Copy link
Collaborator

@heitortsergent heitortsergent left a comment

Choose a reason for hiding this comment

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

Left a comment and a very minor nit, overall LGTM!

One thing, can you please rename the 00 EventBridge.md file to 00 EventBridgeClient.md? It looks like that would fix issue in the preview of the pages being in two different sections.


| Parameter | Type | Description |
| :------------ | :-------------- | :----------------------------------------------------------------------------------------------------------------------- |
| input | [PutEventsInput](#puteventsinput) | An array of objects representing events to be submitted. |
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm looking at both input and Entries with the same description, and I'm wondering if this would be more accurate if it said: "An object with the key Entries and the value of an array of EventBridgeEntry, representing events to be submitted.", or something along these lines.

Wdyt? Or is this more confusing?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see what you mean, but the PutEventsInput is a data interface defined by AWS, which we also define in the code, and which also has other attributes such as EndpointId (and I believe more that we haven't included yet).

Thus, I believe it is better kept this way.

However, I'm not happy with how we deal with these nested type definitions in the docs at the moment. The problem is that if the argument to a function is an object with a strict form and shape that we expect, it's hard to describe it to the user in a concise way. Having multiple tables, as I did here, was my best bet. We could also have dedicated separate pages, but that makes documenting a library like AWS very cumbersome.

What do you think? Maybe you see an alternative that would be more friendly to both the user and the documentation writer? 😄 🙇🏻

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a great point... And I think tables are an ok solution for right now, feel free to merge this PR.

I'll open a separate issue so we can discuss/test some different ways of how to better display nested objects, maybe we can look at some other docs and get some ideas. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Please do so, I'm really keen to work on this with you and think about how we might improve it in the future indeed 👍🏻

Co-authored-by: Heitor Tashiro Sergent <heitortsergent@gmail.com>
@oleiade oleiade merged commit 04c2065 into main Oct 4, 2023
4 checks passed
@oleiade oleiade deleted the aws/EventBridge branch October 4, 2023 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:extensions Issues about the extension ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants