-
-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: initial #1
Conversation
package.json
Outdated
"keywords": [ | ||
"plugin", | ||
"helper", | ||
"fastify" |
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.
"fastify" | |
"fastify", | |
"openapi", | |
"otel", | |
"opentelemetry" |
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.
why openapi
? just out curiosity?
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.
Typo 😄
Let me take the manifest and config from there 👍 |
5a36009
to
da0dc51
Compare
da0dc51
to
1f0230b
Compare
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.
Looks good to me.
README.md
Outdated
[![CI](https://github.com/fastify/otel/actions/workflows/ci.yml/badge.svg?branch=master)](https://github.com/fastify/otel/actions/workflows/ci.yml) | ||
|
||
<!-- [![NPM version](https://img.shields.io/npm/v/@fastify/otel.svg?style=flat)](https://www.npmjs.com/package/fastify-plugin) --> | ||
|
||
[![neostandard javascript style](https://img.shields.io/badge/code_style-neostandard-brightgreen?style=flat)](https://github.com/neostandard/neostandard) |
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.
In my opinion, there's no need to perpetuate these badge things on new repos. For the most part, they were designed to show CI statuses. We use GHA, so such statuses are directly integrated into the repo already (the branch will have a green or red thing with a direct link to the results).
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.
SGTM, I mostly took this directly from fastify-plugin
to keep some sort of standard across the README
; agree with you, I'll remove them away
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.
Looks good to me.
Cross posting this issue for vis, in case it gets carried over open-telemetry/opentelemetry-js-contrib#2619 I'd be happy to help out with a fix once this is merged |
Sounds good! |
Still cannot merge 😅 |
You should be able to now. |
Can we issue a minor release? |
Go for it. |
Checklist
npm run test
andnpm run benchmark
and the Code of conduct