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

Small-Tweaks #869

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Small-Tweaks #869

wants to merge 2 commits into from

Conversation

seanparkross
Copy link
Collaborator

@seanparkross seanparkross commented Feb 3, 2025

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 3, 2025

DX: Assertion Testing

✅ Diff

The PR's diff indicates adjustments to the headers' capitalization and new CORS headers, which improve conventionality and compatibility with cross-origin requests. The explicit error handling and user feedback in the fetchUser function are beneficial additions for better debugging and fault tolerance. The label change from 'Self-hosted' to 'Self-Hosted' in category.json is a minor but good consistency fix, as it adheres to the capitalization convention often used in titles.

❌ Integrated

While the diff's changes make sense individually, there are concerns about how they collectively impact the existing codebase. The introduction of 'mode: cors', 'credentials: include', and broad CORS headers may have security implications requiring further review. Furthermore, the commented out fetchUser call in DocRootLayout/index.js hints at in-progress work, leaving doubt about the readiness of the PR for integration with the other files. Proper verification on the stage environment, as mentioned in the TODO comment, should be conducted before finalizing the PR. Lastly, it's important to ensure that these CORS changes align with the intended access control policy for the application.

@hasura-bot
Copy link
Contributor

@seanparkross Thanks for your PR! I've assigned @robertjdominguez to review it.

Copy link

cloudflare-workers-and-pages bot commented Feb 3, 2025

Deploying ddn-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: f912abf
Status: ✅  Deploy successful!
Preview URL: https://aa83f7b0.v3-docs-eny.pages.dev
Branch Preview URL: https://small-tweaks-20250203.v3-docs-eny.pages.dev

View logs

@seanparkross seanparkross force-pushed the small-tweaks-20250203 branch from 5bf1380 to 53b8d15 Compare February 4, 2025 12:15
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.

2 participants