Skip to content

Conversation

lance
Copy link
Member

@lance lance commented Apr 27, 2020

This commit removes support for the v0.2 specification. It also removes the
contenttype attribute from the CloudEvent object. While the HTTP protocol
binding specifies that in binary mode, the datacontenttype attribute should
map to the HTTP Content-Type header, that doesn't mean that the CloudEvent
object should have a contenttype property.

Fixes: #61
Fixes: #66

Signed-off-by: Lance Ball [email protected]

@lance lance mentioned this pull request Apr 27, 2020
Copy link
Member

@grant grant left a comment

Choose a reason for hiding this comment

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

Looks good besides the massive conflicts.

@@ -91,21 +91,21 @@ These are the supported specifications by this version.

| **Specifications** | v0.1 | v0.2 | v0.3 | **v1.0** |
Copy link
Member

Choose a reason for hiding this comment

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

We should just remove the column for v0.2. We already state we don't support 0.2 above.

@fabiojose
Copy link
Contributor

@lance fix the conflicts, please.

This commit removes support for the v0.2 specification. It also removes the
`contenttype` attribute from the `CloudEvent` object. While the HTTP protocol
binding specifies that in binary mode, the `datacontenttype` attribute should
map to the HTTP Content-Type header, that doesn't mean that the `CloudEvent`
object should have a `contenttype` property.

Fixes: cloudevents#61
Fixes: cloudevents#66

Signed-off-by: Lance Ball <[email protected]>
@lance lance force-pushed the 61-drop-02-support branch from c3a16da to 9edd9cd Compare April 28, 2020 20:20
@lance
Copy link
Member Author

lance commented Apr 28, 2020

@fabiojose conflicts resolved

@fabiojose
Copy link
Contributor

Do we already have the semantic version? Is necessary to fill out the changelog?

@fabiojose fabiojose merged commit 5110ad4 into cloudevents:master Apr 28, 2020
@fabiojose
Copy link
Contributor

fabiojose commented Apr 28, 2020

@lance something goes wrong with CI, can you check?

https://travis-ci.org/github/cloudevents/sdk-javascript/builds/680741903

@grant
Copy link
Member

grant commented Apr 28, 2020

Here's the error:

> [email protected] coverage-publish /home/travis/build/cloudevents/sdk-javascript
> wget -qO - https://coverage.codacy.com/get.sh | sh -s report -l JavaScript -r coverage/lcov.info
sh: 3: set: Illegal option -o pipefail
npm ERR! code ELIFECYCLE
npm ERR! errno 2
npm ERR! [email protected] coverage-publish: `wget -qO - https://coverage.codacy.com/get.sh | sh -s report -l JavaScript -r coverage/lcov.info`
npm ERR! Exit status 2
npm ERR! 
npm ERR! Failed at the [email protected] coverage-publish script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2020-04-28T20_38_50_402Z-debug.log
The command "if [ "$TRAVIS_PULL_REQUEST" == "false" ]; then npm run coverage-publish; else npm test; fi" exited with 2.
cache.2
store build cache
Done. Your build exited with 1.

Something with the coverage-publish script.

@lance lance deleted the 61-drop-02-support branch April 28, 2020 21:14
@lance
Copy link
Member Author

lance commented Apr 28, 2020

@fabiojose @grant This error is confusing. The coverage task didn't change with this commit. In fact, package.json didn't change at all.

Hidden in the error message you pasted above is the real culprit.

sh: 3: set: Illegal option -o pipefail

Here are the first three lines of the codacy https://coverage.codacy.com/get.sh script that is run in order to publish the code coverage stats.

   1   │ #!/usr/bin/env bash
   2   │ 
   3   │ set -e +o pipefail

The error indicates that the get.sh script provided an illegal option to the set command on line 3. Again... seems weird. The .travis.yaml configuration file hasn't changed, so I don't know why all of a sudden the bash set command would begin failing. A simple test with a file like this indicates this should not fail.

   1   │ #!/bin/bash
   2   │ 
   3   │ set -e +o pipefail
   4   │ 
   5   │ echo "hello!"

Maybe the build was run on a corrupted container? I don't have permissions to re-run the build on Travis-CI. But can you try that first?

@fabiojose
Copy link
Contributor

Things went wrong by this PR #49, could you check @lance ?

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.

Cloudevent object should not have a contentype attribute Recommend dropping support for 0.2
3 participants