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.

Sign up for GitHub

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

Jump to bottom

Fix 6206: Cloudflare function path resolving one step above #6208

Merged
merged 4 commits into from
Feb 13, 2023
Merged

Fix 6206: Cloudflare function path resolving one step above #6208

merged 4 commits into from
Feb 13, 2023

Conversation

mfrachet
Copy link
Contributor

@mfrachet mfrachet commented Feb 10, 2023 •
edited
Loading

Fixes #6206

Changes

Make sure the [[path]].js file is generated inside the functions folder in cloudflare.

Testing

Assertion on a test has been added

Docs

I don't think it has an impact

@changeset-bot
Copy link

changeset-bot bot commented Feb 10, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 3cb5935

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Feb 10, 2023
@mfrachet mfrachet marked this pull request as draft February 10, 2023 13:46
@mfrachet mfrachet force-pushed the fix-6206-cloudflare-functions-path branch from 3d9cc0d to b9681c6 Compare February 10, 2023 13:48
@mfrachet mfrachet marked this pull request as ready for review February 10, 2023 14:03
@matthewp
Copy link
Contributor

@AirBorne04 can you review?

@mfrachet Please add a changeset with pnpm changeset, thanks!

AirBorne04
AirBorne04 requested changes Feb 10, 2023
View reviewed changes
Copy link
Contributor

@AirBorne04 AirBorne04 left a comment

Choose a reason for hiding this comment

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

Something weird is ongoing here at least on windows it does not work.

packages/integrations/cloudflare/src/index.ts Outdated Show resolved Hide resolved
packages/integrations/cloudflare/test/directory.test.js Show resolved Hide resolved
@mfrachet mfrachet requested a review from AirBorne04 February 11, 2023 07:48
AirBorne04
AirBorne04 requested changes Feb 11, 2023
View reviewed changes
Copy link
Contributor

@AirBorne04 AirBorne04 left a comment

Choose a reason for hiding this comment

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

I only now had the change to take a good look at the issue, it seems we were only missing a trailing / behind the functions path, fixing this should be sufficient.

packages/integrations/cloudflare/src/index.ts Outdated Show resolved Hide resolved
packages/integrations/cloudflare/src/index.ts Outdated Show resolved Hide resolved
@mfrachet mfrachet requested a review from AirBorne04 February 12, 2023 07:53
AirBorne04
AirBorne04 approved these changes Feb 13, 2023
View reviewed changes
Copy link
Contributor

@AirBorne04 AirBorne04 left a comment

Choose a reason for hiding this comment

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

thank you for the change!

@AirBorne04 AirBorne04 merged commit 79f49ac into withastro:main Feb 13, 2023
@astrobot-houston astrobot-houston mentioned this pull request Feb 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AirBorne04 AirBorne04 AirBorne04 approved these changes

Assignees
No one assigned
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cloudflare mode directory outputs [[path]] to root instead of ./function
3 participants
@mfrachet @matthewp @AirBorne04