Skip to content

Conversation

lance
Copy link
Member

@lance lance commented May 15, 2020

This is a breaking change.

This commit makes a number of changes to the HTTP bindings code in an attempt
to simplify its usage and implementation. From a very high level, this inverts
the existing dependencies.

As an example, consider lib/bindings/http/receiver_structured_1.js.
https://github.com/cloudevents/sdk-javascript/blob/v1.0.0/lib/bindings/http/receiver_structured_0_3.js

This class instantiates lib/bindings/http/receiver_structured.js and delegates
its function invokations to it. This had the effect of requiring a user to know what
event versions they would be receiving. And for me personally was a little confusing
as a maintainer.

The change introduced here reverses that logic, so that the version agnostic receiver
is what the user instantiates. It instantiates the approrpiate version of a specific
receiever and delegates to it - reversing the dependencies.

I've also moved all of the top level directories related to HTTP versions into
lib/bindings/http/v1 and lib/bindings/http/v03 and generally done some rearranging
to make the repository structure cleaner and more organized.

As a side effect:

Fixes: #162

Signed-off-by: Lance Ball lball@redhat.com

@lance lance added type/enhancement New feature or request module/transport/http Issues related to the HTTP transport protocol implementation labels May 15, 2020
@lance lance requested a review from a team May 15, 2020 16:11
@lance lance self-assigned this May 15, 2020
@lance lance requested review from grant, fabiojose and helio-frota and removed request for a team May 15, 2020 16:11
@lance lance force-pushed the simplify-http-receiever-usage branch from f7eb9ca to 8c8cb2e Compare May 15, 2020 16:14
@lance lance mentioned this pull request May 18, 2020
@lance lance force-pushed the simplify-http-receiever-usage branch from f0a2be4 to 7da3127 Compare May 18, 2020 12:54
lance added 2 commits May 18, 2020 09:46
This is a breaking change.

This commit makes a number of changes to the HTTP bindings code in an attempt
to simplify its usage and implementation. From a very high level, this inverts
the existing dependencies.

As an example, consider `lib/bindings/http/receiver_structured_1.js`.
https://github.com/cloudevents/sdk-javascript/blob/v1.0.0/lib/bindings/http/receiver_structured_0_3.js

This class instantiates `lib/bindings/http/receiver_structured.js` and delegates
its function invokations to it. This had the effect of requiring a user to know what
event versions they would be receiving. And for me personally was a little confusing
as a maintainer.

The change introduced here reverses that logic, so that the version agnostic receiver
is what the user instantiates. It instantiates the approrpiate version of a specific
receiever and delegates to it - reversing the dependencies.

I've also moved all of the top level directories related to HTTP versions into
`lib/bindings/http/v1` and `lib/bindings/http/v03` and generally done some rearranging
to make the repository structure cleaner and more organized.

Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
@lance lance force-pushed the simplify-http-receiever-usage branch from 7da3127 to ecd2b2c Compare May 18, 2020 13:46
lance added 2 commits May 18, 2020 09:48
Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
@lance lance requested a review from helio-frota May 18, 2020 15:17
@lance lance merged commit 6f0b5ea into cloudevents:master May 18, 2020
@lance lance deleted the simplify-http-receiever-usage branch May 18, 2020 15:34
<dl class="details">



Copy link
Member

Choose a reason for hiding this comment

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

Why is there all of this whitespace?

@grant
Copy link
Member

grant commented May 18, 2020

Sorry I didn't get to review this in time.

It seems like this PR has a lot of autogenerated HTML that will be in the npm package that isn't useful for consumers. Is that on purpose?

@lance
Copy link
Member Author

lance commented May 18, 2020

Hey @grant. With @helio-frota's PR #147 last week, the package.json has been modified so that it will only include the source code - no docs or tests or examples. The npm run generate-docs generates all of these .html files and they are published here: https://cloudevents.github.io/sdk-javascript/.

@lance
Copy link
Member Author

lance commented May 18, 2020

Sorry I didn't get to review this in time.

😉 this is why I was proposing a grace period - after a PR is reviewed, but before it can be landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/transport/http Issues related to the HTTP transport protocol implementation type/enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add JSDoc to BinaryHTTPReceiver and StructuredHTTPReceiver
3 participants