Skip to content
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

Remove @fastify/accepts dependency #2241

Merged
merged 5 commits into from
Jun 7, 2023
Merged

Remove @fastify/accepts dependency #2241

merged 5 commits into from
Jun 7, 2023

Conversation

Pezmc
Copy link
Contributor

@Pezmc Pezmc commented Jun 6, 2023

Description

The accepts header parsing was previously only used in one place, which actually bypassed most of the parsing.

Now checking for application/openmetrics-text more naively and saving FlowForge one NPM dependency.

Related Issue(s)

N/a

Checklist

  • I have read the contribution guidelines
  • [-] Suitable unit/system level tests have been added and they pass
  • [-] Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Pezmc Pezmc requested a review from knolleary June 6, 2023 12:57
@Pezmc Pezmc changed the title Remove dependencies Remove @fastify/accepts dependency Jun 6, 2023
@knolleary
Copy link
Member

Resolved the merge conflict - okay to merge once tests pass

@Pezmc
Copy link
Contributor Author

Pezmc commented Jun 7, 2023

The merge conflict resolution has re-introduced accept, fixing it now. Good test of our linting though, it caught accepts wasn't included in our package.json.

@knolleary
Copy link
Member

🤦🏻 whilst figuring out what the conflict was I made a mental note to make sure I didn't reintroduce the dependency then got distracted at the vital moment. My apols.

@Pezmc Pezmc merged commit dc0e689 into main Jun 7, 2023
3 of 4 checks passed
@Pezmc Pezmc deleted the remove-dependencies branch June 7, 2023 08:30
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.

None yet

2 participants