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

fix(gatsby): Adapter header rules #38644

Merged
merged 11 commits into from
Oct 24, 2023
Merged

fix(gatsby): Adapter header rules #38644

merged 11 commits into from
Oct 24, 2023

Conversation

kathmbeck
Copy link
Contributor

@kathmbeck kathmbeck commented Oct 18, 2023

Description

Adjustment to reduce the header file size.

Based off of work done by #38561. The primary difference is the equality checks for filtering the headers. includes doesn't work since JS checks for reference equality.

Documentation

Tests

Unit tests

Related Issues

Fixes https://linear.app/netlify/issue/FRA-17/reduce-headers-file-size

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 18, 2023
@kathmbeck kathmbeck changed the title Adapter header rules fix(gatsby): Adapter header rules Oct 19, 2023
@pieh pieh added topic: adapters Related to Gatsby Adapters and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 19, 2023
@kathmbeck kathmbeck self-assigned this Oct 19, 2023
@kathmbeck kathmbeck marked this pull request as ready for review October 19, 2023 14:50
@kathmbeck kathmbeck requested a review from pieh October 19, 2023 14:50
@kathmbeck
Copy link
Contributor Author

Tests were passing, but a new test is failing after merging in master https://app.circleci.com/pipelines/github/gatsbyjs/gatsby/91772/workflows/deb2a349-7db6-4970-965d-01578ec83ea5/jobs/1111499/tests

@kathmbeck
Copy link
Contributor Author

Tests were passing, but a new test is failing after merging in master https://app.circleci.com/pipelines/github/gatsbyjs/gatsby/91772/workflows/deb2a349-7db6-4970-965d-01578ec83ea5/jobs/1111499/tests

Ran again and it passed 🎉

Copy link

@mraerino mraerino 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!
did you want to add an integration test with a dummy adapter maybe?

@kathmbeck
Copy link
Contributor Author

looks good to me! did you want to add an integration test with a dummy adapter maybe?

As discussed in the sync, I'll update the E2E tests in the follow-up PR with the adapter changes.

@pieh pieh merged commit dc21604 into master Oct 24, 2023
1 check passed
@pieh pieh deleted the adapter-header-rules branch October 24, 2023 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: adapters Related to Gatsby Adapters
Projects
Status: Published
Development

Successfully merging this pull request may close these issues.

3 participants