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

Rename attributes 'resource*' to 'source*' #30

Merged
merged 1 commit into from
Feb 1, 2018

Conversation

lfourie
Copy link
Contributor

@lfourie lfourie commented Jan 25, 2018

Closes #38

@lfourie lfourie changed the title Rename attributes resource 'resource*' to 'source*' Rename attributes 'resource*' to 'source*' Jan 25, 2018
@sslavic
Copy link
Contributor

sslavic commented Jan 26, 2018

Have few somewhat related questions:

  1. Does namespace identify event or event source (producer)?
  2. If latter, what is the value namespace adds on top of source-id?
  3. Is there a use case where events with same (event-type, event-type-version) can be published by more than one event source, so with difference in one of (source-id, source-type)?
  4. If namespace identifies event source and there's value added of namespace on top of source-id would it make sense to include namespace together with (source-id, source-type) as property of source object?

@duglin
Copy link
Collaborator

duglin commented Jan 27, 2018

@lfourie you'll need to sign your commit

@duglin
Copy link
Collaborator

duglin commented Jan 27, 2018

@sslavic those are good questions. Can you open a separate issue to answer those since this one is just a syntax/word change and if it is merged quickly I don't want to forget to have a discussion about your questions?

@sslavic
Copy link
Contributor

sslavic commented Jan 27, 2018

Created #41

@duglin
Copy link
Collaborator

duglin commented Jan 27, 2018

@sslavic thanks!

@duglin
Copy link
Collaborator

duglin commented Jan 30, 2018

@lfourie can you please sign your commit - I think we might be able to resolve this one quickly on the next call but we can't merge it unless its signed.

@duglin
Copy link
Collaborator

duglin commented Jan 30, 2018

LGTM once signed

@lfourie lfourie closed this Jan 30, 2018
@sslavic
Copy link
Contributor

sslavic commented Jan 30, 2018

@lfourie why close this one? It hasn't been merged. As @duglin suggested, before it can be merged, commit needs to be amended with sign-off info, as per contributing guidelines https://github.com/cloudevents/spec/blob/master/CONTRIBUTING.md#sign-your-work

@sslavic
Copy link
Contributor

sslavic commented Jan 30, 2018

@lfourie just saw the #43 and that you added a comment there with sign-off info. If I'm not mistaken, sign-off info has to be in the git commit message itself, so once commit is merged sign-off remains in the repo.

@duglin
Copy link
Collaborator

duglin commented Jan 30, 2018

@lfourie I reopened this one and fixed your DCO

@duglin duglin mentioned this pull request Jan 30, 2018
@duglin duglin force-pushed the patch-1 branch 2 times, most recently from 21cad3f to f8a4108 Compare January 30, 2018 21:35
Signed-off-by: lfourie <louis.fourie@huawei.com>
@duglin
Copy link
Collaborator

duglin commented Jan 30, 2018

LGTM

@ultrasaurus
Copy link
Contributor

I like the change from resource => source. I think it is a lot clearer than original naming!

@deissnerk
Copy link
Contributor

LGTM

1 similar comment
@dklyle
Copy link
Contributor

dklyle commented Feb 1, 2018

LGTM

@duglin
Copy link
Collaborator

duglin commented Feb 1, 2018

Approved on Feb 1st call.
AI: Thomas to open issue to reconsider the name
AI: Thomas to work on new wording for definition of “source”

@duglin duglin merged commit 92a24a8 into cloudevents:master Feb 1, 2018
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

Successfully merging this pull request may close these issues.

None yet

6 participants