Skip to content

feat: initial#1

Merged
metcoder95 merged 20 commits into
fastify:mainfrom
metcoder95:feat/initial
Jan 6, 2025
Merged

feat: initial#1
metcoder95 merged 20 commits into
fastify:mainfrom
metcoder95:feat/initial

Conversation

@metcoder95
Copy link
Copy Markdown
Member

@metcoder95 metcoder95 commented Dec 1, 2024

Checklist

Comment thread package.json Outdated
Comment thread package.json Outdated
Comment thread package.json Outdated
"keywords": [
"plugin",
"helper",
"fastify"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
"fastify"
"fastify",
"openapi",
"otel",
"opentelemetry"

Copy link
Copy Markdown
Member Author

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo 😄

@metcoder95
Copy link
Copy Markdown
Member Author

Let me take the manifest and config from there 👍

Copy link
Copy Markdown
Member

@jsumners jsumners 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 to me.

Comment thread README.md Outdated
Comment on lines +3 to +7
[![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)
Copy link
Copy Markdown
Member

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).

Copy link
Copy Markdown
Member Author

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

Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown
Member

@jsumners jsumners 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 to me.

@metcoder95 metcoder95 requested a review from Eomm December 27, 2024 09:47
@drewcorlin1
Copy link
Copy Markdown

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

@metcoder95
Copy link
Copy Markdown
Member Author

Sounds good!
There are a few things that I'd like to add but I believe this should be ok to go, @Eomm is there anything left before approval?
It seems I cannot merge until an approval from a reviewer with write access

@metcoder95
Copy link
Copy Markdown
Member Author

Still cannot merge 😅

@jsumners
Copy link
Copy Markdown
Member

jsumners commented Jan 5, 2025

You should be able to now.

@metcoder95 metcoder95 merged commit 8bd0553 into fastify:main Jan 6, 2025
@metcoder95 metcoder95 deleted the feat/initial branch January 6, 2025 08:48
@metcoder95
Copy link
Copy Markdown
Member Author

Can we issue a minor release?

@jsumners
Copy link
Copy Markdown
Member

jsumners commented Jan 8, 2025

Go for it.

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.

4 participants