Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Upgrade to opentracing-javascript 0.14 #117

Merged
merged 4 commits into from
Apr 21, 2020
Merged

Upgrade to opentracing-javascript 0.14 #117

merged 4 commits into from
Apr 21, 2020

Conversation

yurishkuro
Copy link
Member

There is a potentially breaking change in 0.14

opentracing/opentracing-javascript#79

Need to see how this will affect 0.10 JS code.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 98.498% when pulling c07489d on upgrade-ot-0.14 into 510cebd on master.

@tiffon
Copy link
Member

tiffon commented Apr 24, 2018

Lets upgrade this to 0.14.2... ?

@@ -18,7 +18,7 @@
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
// THE SOFTWARE.

import apiCompatibilityChecks from 'opentracing/lib/test/api_compatibility.js';
import * as apiCompatibilityChecks from 'opentracing/lib/test/api_compatibility.js';

Choose a reason for hiding this comment

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

apiCompatibilityChecks is a default so that makes this alias incorrect unless you use apiCompatibiltiyChecks.default() below: https://github.com/opentracing/opentracing-javascript/blob/master/src/test/api_compatibility.ts#L92

@RohitRox
Copy link

Any update on this?
There are indeed some breaking changes in 0.14.
Weird thing is there is no release around 0.13.0 on github https://github.com/opentracing/opentracing-javascript/releases

It is desirable to update to 0.14 as it seems like it is latest and also got the types for use with typescript.

@yurishkuro
Copy link
Member Author

yurishkuro commented Apr 20, 2020

@RohitRox something was broken in the tests, I do not have time to investigate. I just rebased to latest master. If you have cycles to fix the tests, feel free to grab this branch.

Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro
Copy link
Member Author

Actually, I was able to fix it. Will merge soon.

Signed-off-by: Yuri Shkuro <ys@uber.com>
@RohitRox
Copy link

@RohitRox something was broken in the tests, I do not have time to investigate. I just rebased to latest master. If you have cycles to fix the tests, feel free to grab this branch.

Thanks for getting this into motion. Will be glad to help with anything.

Copy link
Collaborator

@everett980 everett980 left a comment

Choose a reason for hiding this comment

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

package-lock.json needs to have the correct url. otherwise LGTM. to be safe, I'd rm -rf node_modules && yarn/npm install && yarn/npm run build && yarn/npm run test to be certain that the lock manual change is good.

"resolved": "https://registry.npmjs.org/opentracing/-/opentracing-0.13.0.tgz",
"integrity": "sha1-ajQUQvCdfYZrwR7QPeHjgo49aqs="
"version": "0.14.4",
"resolved": "https://unpm.uberinternal.com/opentracing/-/opentracing-0.14.4.tgz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

this registry needs to be manually changed back to registry.npmjs.org

Signed-off-by: Yuri Shkuro <ys@uber.com>
@yurishkuro yurishkuro merged commit 5f0880e into master Apr 21, 2020
@yurishkuro yurishkuro deleted the upgrade-ot-0.14 branch April 21, 2020 17:57
@jaegertracing jaegertracing deleted a comment from codecov bot Apr 21, 2020
Iuriy-Budnikov pushed a commit to agile-pm/jaeger-client-node that referenced this pull request Sep 25, 2021
Change CONTRIBUTING.md
Resolves jaegertracing#117

Signed-off-by: Olivier Albertini <olivier.albertini@montreal.ca>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants