-
Notifications
You must be signed in to change notification settings - Fork 225
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
ci(local-npm): resume integration testing of NPM canary #7883
Changes from all commits
ca530f8
b4a5984
f33f4e2
9364d7d
ff85a7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,8 @@ import { | |
|
||
// diag.setLogger(new DiagConsoleLogger(), DiagLogLevel.VERBOSE); | ||
|
||
/** @typedef {import('@opentelemetry/api-metrics').MetricAttributes} Attributes */ | ||
/** @typedef {import('@opentelemetry/api-metrics').Histogram} Histogram */ | ||
/** @typedef {import('@opentelemetry/api').MetricAttributes} Attributes */ | ||
/** @typedef {import('@opentelemetry/api').Histogram} Histogram */ | ||
|
||
import { getTelemetryProviders as getTelemetryProvidersOriginal } from '@agoric/telemetry'; | ||
|
||
|
@@ -111,7 +111,7 @@ export function getTelemetryProviders(powers = {}) { | |
} | ||
|
||
/** | ||
* @param {import('@opentelemetry/api-metrics').Meter} metricMeter | ||
* @param {import('@opentelemetry/api').Meter} metricMeter | ||
* @param {string} name | ||
*/ | ||
function createHistogram(metricMeter, name) { | ||
|
@@ -121,8 +121,8 @@ function createHistogram(metricMeter, name) { | |
|
||
/** | ||
* @param {{ | ||
* metricMeter: import('@opentelemetry/api-metrics').Meter, | ||
* attributes?: import('@opentelemetry/api-metrics').MetricAttributes, | ||
* metricMeter: import('@opentelemetry/api').Meter, | ||
* attributes?: import('@opentelemetry/api').MetricAttributes, | ||
* }} param0 | ||
*/ | ||
export function makeSlogCallbacks({ metricMeter, attributes = {} }) { | ||
|
@@ -284,7 +284,7 @@ export function makeInboundQueueMetrics(initialLength) { | |
/** | ||
* @param {object} param0 | ||
* @param {any} param0.controller | ||
* @param {import('@opentelemetry/api-metrics').Meter} param0.metricMeter | ||
* @param {import('@opentelemetry/api').Meter} param0.metricMeter | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this commit is titled "fix(cosmic-swingset): adapt to new @opentelemetry". For the changelog, there is nothing to fix. This is really a fixup to the change that bumped the OpenTelemetry version. Better to squash it into that commit. Even better is isolating the opentelemetry changes (deps and code) from the other commits. |
||
* @param {Console} param0.log | ||
* @param {Attributes} [param0.attributes] | ||
* @param {any} [param0.inboundQueueMetrics] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,7 +41,8 @@ | |
}, | ||
"files": [ | ||
"*.js", | ||
"NEWS.md" | ||
"NEWS.md", | ||
"src" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could we do a build instead so we're not back to exporting all the modules? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see how this is any worse than the status quo. This package.json is not viable: it produces a tarball that doesn't have any implementation. That broke the integration test. What kind of build do you have in mind? If you just care about exports, let's add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It's net better, for sure. But it fails at something that was intended and I wonder if we can make it work as intended. I had in mind a |
||
], | ||
"publishConfig": { | ||
"access": "public" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,6 +65,8 @@ | |
"files": [ | ||
"CHANGELOG.md", | ||
"src/", | ||
"scripts/", | ||
"tools/", | ||
"*.json", | ||
"globals.d.ts", | ||
"exported.js" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,7 @@ | |
"files": [ | ||
"LICENSE*", | ||
"api.js", | ||
"build.env", | ||
"src" | ||
], | ||
"publishConfig": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
"build": "yarn build:bundles", | ||
"build:bundles": "node scripts/build-bundles.js", | ||
"prepack": "echo \"export {}; \" | cat - tools/types-ambient.js > tools/types.js && tsc --build jsconfig.build.json", | ||
"postpack": "git clean -f '*.d.ts*'", | ||
"postpack": "git clean -f '*.d.ts*' tools/types.js", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is more correct. was it also part of an observed problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, leaving behind garbage in the repo after |
||
"test": "ava --verbose", | ||
"test:c8": "c8 $C8_OPTIONS ava --config=ava-nesm.config.js", | ||
"test:unit": "ava 'test/unitTests/**/test-*.js' -T 1m --verbose", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't actually exercise the same process as
lerna publish
, so I'm disabling it in favour of the integration test.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the explanation