chore: remove stale FIXME comments on snapshot URL - #276
teyrebaz33 wants to merge 1 commit into
Conversation
|
Looked into splitting the label per your nit — turns out there's no clean way to do it. Traced |
|
LGTM — this matches the #275 recommendation precisely. Change is correct and complete:
Merge status (head Ultra-minor, non-blocking: the two replacement comments are worded slightly differently — L26 "Base prefix for the object store; the resolvable pointer is Nice, clean fix — resolves #275 as intended. |
Both FIXMEs read 'Update this to the actual snapshot URL', but the URLs are already the real, working snapshot endpoints — confirmed via crates/snapshots/src/download.rs using the same host without any such disclaimer, and docs/running-an-arc-node.md documenting it directly to users with no caveat. Replaced with a note on the base-prefix/latest.txt object-store pattern per @osr21's review on circlefin#275, to pre-empt someone re-adding a FIXME after curling the bare base URL and getting a 404. Fixes circlefin#275
af6f6ff to
c615888
Compare
|
Hi @teyrebaz33, Thank you for your interest in contributing to Arc Node, and apologies for the delay in getting back to this PR. We're closing out the pull request backlog that predates our current contribution policy. This PR is being closed because the issue it references (#275) is already closed. All PRs must reference an existing issue using the format This is not a judgement on the change itself. If you'd still like to land it:
Please see CONTRIBUTING.md for details. Thanks again for taking the time to contribute. |
Fixes #275
Both FIXMEs in
crates/execution-config/src/defaults.rsread 'Update this to the actual snapshot URL', but the URLs are already the real, working snapshot endpoints — confirmed viacrates/snapshots/src/download.rsusing the same host without any such disclaimer, anddocs/running-an-arc-node.mddocumenting it directly to users with no caveat.Per @osr21's review on #275: verified
GET https://snapshots.arc.network/5042002/latest.txtreturns 200 with a live testnet snapshot pointer, confirming the base URL is correct and object keys (not the bare prefix) are what resolve.Replaced both FIXMEs with a note on the base-prefix/
latest.txtpattern, so a future curl-the-bare-URL-get-a-404 doesn't lead someone to re-add a FIXME.Left the
(testnet)/(devnet)label-in-URL-string nit onavailable_snapshotsuntouched — it's display-only today per the review, and splitting it out felt like a separate, unrelated change to bundle into a stale-comment cleanup.