Repository navigation
lib!: refactor HTTP bindings and specifications - #165
Merged
Merged
Conversation
lance
requested review from
fabiojose,
grant and
helio-frota
and removed request for
a team
May 15, 2020 16:11
lance
force-pushed
the
simplify-http-receiever-usage
branch
from
May 15, 2020 16:14
f7eb9ca to
8c8cb2e
Compare
Closed
helio-frota
approved these changes
May 18, 2020
lance
force-pushed
the
simplify-http-receiever-usage
branch
from
May 18, 2020 12:54
f0a2be4 to
7da3127
Compare
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
force-pushed
the
simplify-http-receiever-usage
branch
from
May 18, 2020 13:46
7da3127 to
ecd2b2c
Compare
Signed-off-by: Lance Ball <lball@redhat.com>
Signed-off-by: Lance Ball <lball@redhat.com>
helio-frota
approved these changes
May 18, 2020
grant
reviewed
May 18, 2020
| <dl class="details"> | ||
|
|
||
|
|
||
|
|
Member
There was a problem hiding this comment.
Why is there all of this whitespace?
Member
|
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? |
Member
Author
|
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 |
Member
Author
😉 this is why I was proposing a grace period - after a PR is reviewed, but before it can be landed. |
Closed
Closed
Closed
This was referenced Apr 16, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.jsand delegatesits 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/v1andlib/bindings/http/v03and generally done some rearrangingto make the repository structure cleaner and more organized.
As a side effect:
Fixes: #162
Signed-off-by: Lance Ball lball@redhat.com