-
-
Notifications
You must be signed in to change notification settings - Fork 5
Update OneDollarStats script handling #698
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -58,6 +58,7 @@ const SITE_URL = "https://osschat.dev"; | |||||||||||||||
| const SITE_NAME = "osschat"; | ||||||||||||||||
| const SITE_DESCRIPTION = "Open source AI chat with 350+ models. Access GPT-4, Claude, Gemini, and more through one beautiful interface. Free tier available, no API key required."; | ||||||||||||||||
| const SITE_TAGLINE = "One interface. Every AI model."; | ||||||||||||||||
| const ONEDOLLARSTATS_SCRIPT_SRC = "https://assets.onedollarstats.com/stonks.js"; | ||||||||||||||||
|
|
||||||||||||||||
| export const Route = createRootRoute({ | ||||||||||||||||
| beforeLoad: async () => { | ||||||||||||||||
|
|
@@ -158,10 +159,9 @@ export const Route = createRootRoute({ | |||||||||||||||
| }, | ||||||||||||||||
| // Analytics | ||||||||||||||||
| { | ||||||||||||||||
| src: "https://assets.onedollarstats.com/stonks.js", | ||||||||||||||||
| src: ONEDOLLARSTATS_SCRIPT_SRC, | ||||||||||||||||
| defer: true, | ||||||||||||||||
| crossOrigin: "anonymous", | ||||||||||||||||
| integrity: "sha384-JKNAwAZy8iZWcJrexWvQf3rNcosuH0th/rwqZoiM84ea7fMGTt2eq8ddZb//nd9H", | ||||||||||||||||
| }, | ||||||||||||||||
|
Comment on lines
+162
to
165
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Confidence this is worth changing: 2/5 — it's harmless in practice, but keeping it is misleading and suggests SRI is still in play.
Suggested change
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! |
||||||||||||||||
| ], | ||||||||||||||||
| }), | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description claims
ONEDOLLARSTATS_SCRIPT_INTEGRITYwas also extracted — it wasn'tThe PR summary states:
And the auto-generated cubic summary echoes:
However, the diff only adds
ONEDOLLARSTATS_SCRIPT_SRC. Theintegrityattribute was removed entirely rather than being extracted to a named constant. If the intent was truly to make the hash easy to update in one place, the constant should have been created and wired back in — otherwise the description misrepresents the change.If the intent was to drop SRI entirely (a valid decision, discussed in the previous thread), the PR description should be updated to reflect that.
Confidence: 3/5 — the description / code mismatch is real, but the behaviour itself (removing SRI) may be intentional even if the words say otherwise.