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 files #533

Merged
merged 1 commit into from
Oct 15, 2019
Merged

Rename files #533

merged 1 commit into from
Oct 15, 2019

Conversation

duglin
Copy link
Collaborator

@duglin duglin commented Oct 10, 2019

if #521 and #529 go in, then rename for consistency

Just look at the last commit to see the changes specific to this PR and not #521 and #529

@duglin duglin added the v1.0 label Oct 10, 2019
@duglin duglin force-pushed the renameFiles branch 2 times, most recently from 91d5a20 to d27eaf3 Compare October 14, 2019 15:48
@duglin
Copy link
Collaborator Author

duglin commented Oct 14, 2019

CI errors are expected until merged.
Ready for review

@markpeek
Copy link
Contributor

LGTM

Copy link
Contributor

@deissnerk deissnerk left a comment

Choose a reason for hiding this comment

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

Overall LGTM
I suggested some change of wording, that could also go into a separate PR, if you want to restrict this one to simple renaming.

@@ -30,7 +30,7 @@ This document is a working draft.

## 1. Introduction

[CloudEvents][ce] is a standardized and transport-neutral definition of the
[CloudEvents][ce] is a standardized and protocol-neutral definition of the
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder, if protocol-agnostic sounds better, but I leave it up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok I got all of these in all of the files

avro-format.md Outdated
@@ -18,7 +18,7 @@ This document is a working draft.

## 1. Introduction

[CloudEvents][ce] is a standardized and transport-neutral definition of the
[CloudEvents][ce] is a standardized and protocol-neutral definition of the
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@@ -35,8 +35,8 @@ those events.
The partitionkey attribute extension uses the key `partitionkey` for
in-memory formats.

### Transport format
### Protocol format
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if in-memory format vs. protocol format are still the right categories. What about stand-alone event format and protocol binding? Or structured-mode message vs. binary-mode message?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This feels like more than a syntax fix so I'd prefer to leave that for a follow-on PR - if you want to open that one up

Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Collaborator Author

duglin commented Oct 15, 2019

Got 2 LGTMs and since this is just syntax fixes I'm going to merge. If anyone thinks there's something I missed or that a change was made incorrectly please raise an issue and we'll address it immediately.

@duglin duglin merged commit f5d9930 into cloudevents:master Oct 15, 2019
@duglin duglin deleted the renameFiles branch October 15, 2019 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants