Skip to content

lib!: refactor HTTP bindings and specifications #165

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

Merged
merged 4 commits into from
May 18, 2020

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