Skip to content

Conversation

@avivkeller
Copy link
Member

We use this in several places, so it should be a constant, no?

https://en.wikipedia.org/wiki/Don%27t_repeat_yourself

Copilot AI review requested due to automatic review settings May 3, 2025 17:20
@avivkeller avivkeller requested a review from a team as a code owner May 3, 2025 17:20
@vercel
Copy link

vercel bot commented May 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
nodejs-org ✅ Ready (Inspect) Visit Preview May 3, 2025 6:32pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new constant, IS_NOT_VERCEL_RUNTIME_ENV, to centralize the runtime environment check logic and eliminate duplicate condition evaluations across multiple files. Key changes include:

  • Adding the new constant in apps/site/next.constants.mjs.
  • Removing duplicate runtime environment checks in releaseData.ts, downloadSnippets.ts, and blogData.ts by importing the new constant.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
apps/site/next.constants.mjs Adds the IS_NOT_VERCEL_RUNTIME_ENV constant to centralize env checks.
apps/site/next-data/releaseData.ts Removes local runtime env condition declaration in favor of the constant.
apps/site/next-data/downloadSnippets.ts Removes local runtime env condition declaration in favor of the constant.
apps/site/next-data/blogData.ts Removes local runtime env condition declaration in favor of the constant.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Aviv Keller <me@aviv.sh>
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 74.85%. Comparing base (2f9f1b2) to head (60e8292).
Report is 6 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
apps/site/next.constants.mjs 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7721      +/-   ##
==========================================
+ Coverage   74.82%   74.85%   +0.02%     
==========================================
  Files          98       98              
  Lines        7878     7890      +12     
  Branches      199      200       +1     
==========================================
+ Hits         5895     5906      +11     
- Misses       1982     1983       +1     
  Partials        1        1              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Aviv Keller <me@aviv.sh>
@avivkeller
Copy link
Member Author

Requesting fast track. if you approve, merge when ready

@github-actions
Copy link
Contributor

github-actions bot commented May 3, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 99 🟢 100 🟢 100 🟢 91 🔗
/en/about 🟢 100 🟢 100 🟢 100 🟢 91 🔗
/en/about/previous-releases 🟢 98 🟢 100 🟢 100 🟠 83 🔗
/en/download 🟢 98 🟢 100 🟢 100 🟢 91 🔗
/en/blog 🟢 100 🟢 100 🟢 96 🟢 92 🔗

@ovflowd ovflowd added this pull request to the merge queue May 3, 2025
Merged via the queue into main with commit 581c972 May 3, 2025
21 checks passed
@ovflowd ovflowd deleted the chore/add-not-vercel-runtime-env branch May 3, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fast-track Fast Tracking PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants