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

Remove lower-case restriction, lint. #522

Merged
merged 3 commits into from
Oct 10, 2019
Merged

Conversation

n3wscott
Copy link
Member

@n3wscott n3wscott commented Oct 4, 2019

  • removed Attribute names SHOULD begin with a lower-case letter as it was redundant.
  • Ran lint.

@duglin duglin added the v1.0 label Oct 7, 2019
@duglin
Copy link
Collaborator

duglin commented Oct 7, 2019

LGTM

if I can get one more LGTM we can merge since this is strictly syntax/typo fixes.

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.

I found one more typo in a part of the original spec, which has been touched by this PR. Apart from that, LGTM

spec.md Outdated
All content attributes MUST be of scalar type (e.g. string, integer)
that have a string-encoding defined. They MUST NOT be of complex type
(e.g. structures, map).
All content attributes MUST be of scalar type (e.g. string, integer) that have a
Copy link
Contributor

Choose a reason for hiding this comment

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

s/content/context

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good eye!
@n3wscott want to get this one added to the PR?

@duglin
Copy link
Collaborator

duglin commented Oct 8, 2019

rebase needed

Scott Nichols and others added 3 commits October 9, 2019 23:42
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Scott Nichols <nicholss@google.com>
Signed-off-by: Doug Davis <dug@us.ibm.com>
@duglin
Copy link
Collaborator

duglin commented Oct 9, 2019

ok - per Scott's request I rebased and made @deissnerk's s/content/context/ typo fix.

Can I get one pair of eyes to review to make sure I didn't introduce another typo... then we can merge since it's just a minor typo type of PR?

@deissnerk
Copy link
Contributor

LGTM

@duglin
Copy link
Collaborator

duglin commented Oct 10, 2019

thanks @deissnerk
merging since it's in the "typos & syntax clean-up" category

@duglin duglin merged commit 5aa00c3 into cloudevents:master Oct 10, 2019
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