kamilkisiela / release-canary

MIT License
3 stars 3 forks source link

Verify and report whether the changesets are valid #12

Closed n1ru4l closed 2 years ago

n1ru4l commented 2 years ago

Some people don't want to use the changeset cli (idk why) and forget to add the version bump header at the start of the file. This error/issue is currently swallowed. The release-canary action should be in charge of detecting this and fail early + comment this on the PR.

Logs from https://github.com/dotansimha/graphql-yoga/runs/6880849860?check_suite_focus=true

Running "Release Canary Version" action...
Using "changesets" for publishing...
/usr/local/bin/yarn release:canary
yarn run v1.22.18
$ (node scripts/canary-release.js && yarn build && yarn changeset publish --tag canary) || echo Skipping Canary...
Error: could not parse changeset - invalid frontmatter: ## Multiple parameters are not recommended (not used internally) for log methods
Previously sometimes Yoga used to send data to the provided logger like below;
```js
yogaLogger.debug(arg1, arg2, arg3)

This behavior is working fine with JavaScript's native console implementation but most of the other non native logger implementation like Pino only accept a single parameter for its logging methods. So we decided to avoid sending multiple parameters to the logger. However, in order to prevent a breaking change, we kept the signatures of logger methods and they will still accept multiple parameters in a single call. You should keep on mind that eventually we will stop accepting multiple parameters and have the behavior similar to Pino's.

Note for custom logger and fastify users

We still recommend to update your logging parameter in createServer call to make sure the other parameters after the first one aren't ignored if exists;

createServer({
  ...someOtherOptions,
  logging: {
    // app.log is Fastify's logger
    // You should replace it with your own if you have some other logger implementation
    debug: (...args) => args.forEach((arg) => app.log.debug(arg)),
    info: (...args) => args.forEach((arg) => app.log.info(arg)),
    warn: (...args) => args.forEach((arg) => app.log.warn(arg)),
    error: (...args) => args.forEach((arg) => app.log.error(arg)),
  },
})

No more custom inspect

Previously Yoga's default logger implementation was using a platform independent port of Node's util.inspect. It was helping us to mimic console.log's behavior to serialize object in a pretty way. But we no longer use it and pass multiple parameters to console.log/debug/info/error instead and leave the serialization to the environment. Don't get confused with the one above :) This is an optimization with default console which already supports multiple values. But the improvement above is for non native logger implementations. at Object.parseChangesetFile [as default] (/home/runner/work/graphql-yoga/graphql-yoga/node_modules/@changesets/parse/dist/parse.cjs.dev.js:16:11) at /home/runner/work/graphql-yoga/graphql-yoga/node_modules/@changesets/read/dist/read.cjs.dev.js:137:71 at async Promise.all (index 0) at async getChangesets (/home/runner/work/graphql-yoga/graphql-yoga/node_modules/@changesets/read/dist/read.cjs.dev.js:141:48) at async updateVersions (/home/runner/work/graphql-yoga/graphql-yoga/scripts/canary-release.js:46:23) Skipping Canary... Done in 0.55s. Matching in line content "yarn run v1.22.18", result is: "null" Matching in line content "$ (node scripts/canary-release.js && yarn build && yarn changeset publish --tag canary) || echo Skipping Canary...", result is: "null" Matching in line content "Skipping Canary...", result is: "null" Matching in line content "Done in 0.55s.", result is: "null" Matching in line content "", result is: "null" No packages were published...

n1ru4l commented 2 years ago

I also found another issue that failed silently:

Error: Found mixed changeset witty-boats-know
Found ignored packages: @envelop-examples/apollo-server @envelop-examples/azure-functions @envelop-examples/cloudflare-workers @envelop-examples/express-graphql @envelop-examples/google-cloud-functions @envelop-examples/graphql-helix @envelop-examples/graphql-helix-auth0 @envelop-examples/graphql-socket.io @envelop-examples/graphql-sse @envelop-examples/graphql-ws @envelop-examples/lambda-aws @envelop-examples/nexus @envelop-examples/simple-http @envelop-examples/typegraphql @envelop-examples/with-esm website
Found not ignored packages: @envelop/response-cache @envelop/core @envelop/apollo-datasources @envelop/apollo-federation @envelop/apollo-server-errors @envelop/apollo-tracing @envelop/auth0 @envelop/dataloader @envelop/depth-limit @envelop/disable-introspection @envelop/execute-subscription-event @envelop/extended-validation @envelop/filter-operation-type @envelop/fragment-arguments @envelop/generic-auth @envelop/graphql-jit @envelop/graphql-middleware @envelop/graphql-modules @envelop/live-query @envelop/newrelic @envelop/opentelemetry @envelop/operation-field-permissions @envelop/parser-cache @envelop/persisted-operations @envelop/preload-assets @envelop/prometheus @envelop/rate-limiter @envelop/resource-limitations @envelop/response-cache-redis @envelop/sentry @envelop/statsd @envelop/validation-cache @envelop/testing @envelop/types
Mixed changesets that contain both ignored and not ignored packages are not allowed

I guess it makes the most sense to fail if the command exits with a non-0 exit code.