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

Configure fastly cleanup #8507

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Configure fastly cleanup #8507

wants to merge 12 commits into from

Conversation

cwillisf
Copy link
Contributor

@cwillisf cwillisf commented Jul 8, 2024

Changes:

  • Convert routes.json to routes.js and move conditional logic into that shared file so that all scripts that process routes can be on the same page (heh) about what routes exist. For example, webpack and configure-fastly.js will now always see the same routes given the same environment variables.
  • Rebuild fastly-extended.js with correct JSDoc type hints
    • Note that the type hints in node_modules/fastly/lib/index.js are malformed. Newer versions are correct, but present a VERY different API.
  • Fix error handling in configure-fastly.js and friends
    • the async library expects Error objects, not strings
    • some call-sites were passing err, res style arguments to a function that only takes err
  • Add a mocking & logging setup for Fastly API requests

These changes should make it easier to understand our routing and Fastly configuration, but should not cause any significant difference in the actual configuration.

This is the foundation for an upcoming functional change to configure-fastly.js

Test Coverage:

I ran this locally and the output seems reasonable, but I'd need to compare it to the real Fastly configuration to be sure. On the plus side, this will provide a baseline so it's easier to tell in the future if a change has affected the configure-fastly.js output. Existing tests pass.

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.

1 participant