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.

Sign up for GitHub

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

Jump to bottom

Renaming/broadening "source" to "topic", consolidating source-id/source-type into "subject" #95

Closed
wants to merge 4 commits into from
Closed

Renaming/broadening "source" to "topic", consolidating source-id/source-type into "subject" #95

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
12a21f6
Renaming source-id to subject-id, dropping source-type
Feb 21, 2018
34e7944
Update spec.md
clemensv Feb 21, 2018
5cb21b9
Update spec.md
clemensv Feb 21, 2018
0dfb77a
Additionally renaming and rephrasing "source"
clemensv Mar 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,24 +154,23 @@ that contains both context and data).
* REQUIRED
* MUST contain at least one non-empty sub-property.

### source-type
### subject
* Type: String
* Description: Type of the event source. Providers define list of event
sources.
* Description: Identifies the subject of the event in relation to the source.
For example, a notification about a new customer record being added to a CRM
system might identify the CRM system as the "source", might further qualify the
event as 'new-customer-added' in the "event-type" relative to its "namespace",
and then further qualify the subject of the event (the new record) with the content
of this property. The subject is a free-form string defined by the publisher. The
property is optional, because the subject might already be self-evident from
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps: The property might not be present, because the subject...
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a bit weird to tap dance around OPTIONAL here. Shall I uppercase or move it back?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the bigger concern, at least to me, is saying its OPTIONAL twice - once here and once below. Duplicating normative statements isn't good, its opening the possibility of someone updating one of them but not the other - then we're inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we have a constraints section, we should also allow rationalization of the constraint in that section, IMO. I'm moving it back.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't necessary object to that - I'm just trying to avoid two sections that give lots of explanatory text and people not knowing what text goes where - so they end up duplicating thoughts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, on a side note, this is partially why I tend to prefer to not have a "constraints" section because it forces this kind of split. I'd prefer just a set of sentences with the RFC keywords in the appropriate spots and they can live next to the explanatory text. But I didn't want to drastically change the original format of the spec when we first pulled it in. But if people are open to the change I can take a pass at a PR.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ping @ac360 - any thoughts on this?

Copy link
Contributor

@deissnerk deissnerk Feb 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Providing descriptions without mentioning the constraints will in many cases be hard. @duglin I agree with you that a split of the text is hard to achieve. On the other hand I see some value in a restrictions section that only lists the restrictions without providing additional text. An implementor of the spec who just wants to quickly check the restrictions of an attribute, might benefit from that. Using the same normative statements in the description would be even better than paraphrasing them, because it would help maintaining consistency between description and constraints.

the "source" context.
* Constraints:
* REQUIRED
* MUST be a non-empty string
* Examples:
* s3

### source-id
* Type: String
* Description: ID of the event source.
* Constraints:
* REQUIRED
* MUST be a non-empty string
* OPTIONAL
* Examples:
* my.s3.bucket
* File system event, a newly created file being the subject, with a partial URI
relative to the source URI: /data/app/alice.txt
* Customer relationship management (CRM) solution event, new customer record being
the subject, with fully qualified URI: https://example.com/crm/c/2662773

### event-id
* Type: String
Expand Down