Skip to content

Rethink Versioning #132

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

Closed
grant opened this issue May 6, 2020 · 9 comments
Closed

Rethink Versioning #132

grant opened this issue May 6, 2020 · 9 comments

Comments

@grant
Copy link
Member

grant commented May 6, 2020

When a developer wants to use CloudEvents, they think about an event, not which version they're using. The way this module is written is highly segmented towards the version of CloudEvent.

Rather than this:

const v1 = require("cloudevents-sdk/v1");

Where const v1 doesn't make much sense.

We should have

const CloudEvents = require("cloudevents-sdk");

It should be easy to create a CloudEvent:

let myevent= new CloudEvent({
  specversion: '1.0',
  source: '/source',
  type: 'type',
  dataContentType: 'text/plain',
  dataschema: 'http://d.schema.com/my.json',
  subject: 'cha.json',
  data: 'my-data',
  addExtension: ["my-ext", "0x600"]
});

let myevent2= new CloudEvent({
  specversion: '0.3',
  source: '/source',
  type: 'type',
  dataContentType: 'text/plain',
  dataschema: 'http://d.schema.com/my.json',
  subject: 'cha.json',
  data: 'my-data',
  addExtension: ["my-ext", "0x600"]
});

We should remove the v03 and v1 folders in the repo and just have one CloudEvent that can support multiple versions.

@lance
Copy link
Member

lance commented May 7, 2020

That's in some ways what this pull request that landed yesterday was about.

const { CloudEvent, HTTPReceiver } = require('cloudevent-sdk');
const ce = new CloudEvent();
// Produces
// CloudEvent {
//  spec:
//   Spec1 {
//     payload:
//      { specversion: '1.0',
//        id: '888798a5-0a22-463a-b2d5-bd9a443c736b' },
//     caller: [Function: CloudEvent] },
//  formatter: JSONFormatter {},
//  extensions: {} }

And you can see the HTTPReceiver usage in the test here: https://github.com/cloudevents/sdk-javascript/blob/master/test/bindings/http/promiscuous_receiver_test.js#L13

Once a CloudEvent has been created, that instance cannot be changed to a different version by just setting the specversion property. You can see here, that was a conscious choice by @fabiojose and I think it makes sense. Converting from one specification version to another is more than just changing the version number.

With the changes that landed yesterday, when a CloudEvent is created using the constructor from the top level default export, it's defaulting to an event conforming to the 1.0 spec. You can create a 0.3 event by reaching down into the v3 exports as before.

I think your direct object notation could be a nice change. But that's already covered by #65 isn't it?

@grant
Copy link
Member Author

grant commented May 7, 2020

Glad we're making progress.
Yeah, changing the CloudEvent version isn't just changing the specversion of course. But there are more things similar between versions than there are that are different.

#65 should make this much easier, yes.

@lance
Copy link
Member

lance commented May 7, 2020

It seems like #65 should cover what you are asking for here right? Can this be closed?

@lance
Copy link
Member

lance commented May 7, 2020

Also related #124

@grant
Copy link
Member Author

grant commented May 7, 2020

It seems like #65 should cover what you are asking for here right? Can this be closed?

No, they are separate. The non-builder pattern won't fix const v1 = require("cloudevents-sdk/v1");. I gave an example of what it would look like that isn't covered by other issues.

@lance
Copy link
Member

lance commented May 7, 2020

Oh, I must be confused then. I was thinking you wanted, in addition to the direct object notation, something that looks like this.

const { CloudEvent, HTTPReceiver } = require('cloudevent-sdk');
const ce = new CloudEvent();

That's what just landed recently. What else are you thinking here?

@grant
Copy link
Member Author

grant commented May 7, 2020

Oh, I must be confused then. I was thinking you wanted, in addition to the direct object notation, something that looks like this.

const { CloudEvent, HTTPReceiver } = require('cloudevent-sdk');
const ce = new CloudEvent();

That's what just landed recently. What else are you thinking here?

That looks like it solves the issue. If it landed, that should fix the issue. I think the README is out of date from the implementation then. Or perhaps both styles work now.

@lance
Copy link
Member

lance commented May 7, 2020

Yes it is. I added this issue earlier this week to deal with that before releasing 2.0.0.

@lance
Copy link
Member

lance commented May 12, 2020

Closing this, as I think we have come to resolution. If not, feel free to reopen and we can discuss further.

@lance lance closed this as completed May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants