Skip to content

Remove accidental whitelist changes#6524

Merged
Alek99 merged 1 commit into
mainfrom
masenf/unwhitelist
May 18, 2026
Merged

Remove accidental whitelist changes#6524
Alek99 merged 1 commit into
mainfrom
masenf/unwhitelist

Conversation

@masenf
Copy link
Copy Markdown
Collaborator

@masenf masenf commented May 18, 2026

Leave the commented out code so it's easy to just compile the getting started page for testing purposes.

Leave the commented out code so it's easy to just compile the getting started
page for testing purposes.
@masenf masenf requested review from a team and Alek99 as code owners May 18, 2026 21:46
@Alek99 Alek99 merged commit 5eaaff7 into main May 18, 2026
69 checks passed
@Alek99 Alek99 deleted the masenf/unwhitelist branch May 18, 2026 21:47
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 18, 2026

Greptile Summary

This PR reverts two accidental entries (/library/forms/select and /library/forms/form) that were added to WHITELISTED_PAGES in a previous commit, restoring the list to an effectively empty/all-pages state.

  • The only remaining entry, # "/getting-started/introduction", is intentionally left as a comment so developers can quickly uncomment it when testing a local build against that single page. This conflicts with the team rule requiring commented-out code to be removed before merging.

Confidence Score: 4/5

Safe to merge — the only functional change is removing two stale whitelist entries, returning the list to an empty (build-all) state. The commented-out line is cosmetic.

The revert itself is correct and low-risk. The one open question is whether the deliberately committed comment aligns with team practice; otherwise the change is straightforward.

docs/app/reflex_docs/whitelist.py — contains a commented-out entry that may warrant cleanup per team convention.

Important Files Changed

Filename Overview
docs/app/reflex_docs/whitelist.py Reverts accidental additions of /library/forms/select and /library/forms/form to WHITELISTED_PAGES; keeps /getting-started/introduction as a commented-out entry for developer convenience, which conflicts with the team rule against committed commented-out code.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Request for page path] --> B{WHITELISTED_PAGES empty?}
    B -- Yes --> C[Build all pages ✓]
    B -- No --> D{path == /}
    D -- Yes --> E[Always build root ✓]
    D -- No --> F{Only entry is /}
    F -- Yes --> G[Skip page ✗]
    F -- No --> H{path.startswith any whitelisted?}
    H -- Yes --> I[Build page ✓]
    H -- No --> J[Skip page ✗]
Loading

Reviews (1): Last reviewed commit: "Remove accidental whitelist changes" | Re-trigger Greptile

"/getting-started/introduction",
"/library/forms/select",
"/library/forms/form",
# "/getting-started/introduction",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Commented-out code left in production file

The team's rule requires removing commented-out code before merging. The PR description acknowledges this is intentional for local testing convenience, but the commented-out entry # "/getting-started/introduction" will persist in the main branch and can cause confusion about whether this page is intentionally excluded or accidentally left commented out. Consider removing the comment and relying on the module docstring (which already includes this path as an example) to remind developers how to re-enable a page for testing.

Rule Used: Remove commented-out code before merging PRs. (source)

Learned From
reflex-dev/reflex-web#1619

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 18, 2026

Merging this PR will not alter performance

✅ 24 untouched benchmarks


Comparing masenf/unwhitelist (659fa77) with main (ada9e10)

Open in CodSpeed

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