Opened 5 years ago
Closed 5 years ago
#19349 closed enhancement (fixed)
Apply #18731 again
Reported by: | jdemeyer | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | sage-6.10 |
Component: | build | Keywords: | |
Cc: | jhpalmieri | Merged in: | |
Authors: | Jeroen Demeyer | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | 9258278 (Commits) | Commit: | 92582788c1f3a01479f4414b466275120aa843ec |
Dependencies: | Stopgaps: |
Description (last modified by )
Change History (11)
comment:1 Changed 5 years ago by
- Cc jhpalmieri added
comment:2 Changed 5 years ago by
- Branch set to u/jdemeyer/apply__18731_again
comment:3 Changed 5 years ago by
- Commit set to 5a0c4b0d9610c2eee9f8d25f929ae3047cb65df8
- Status changed from new to needs_review
comment:4 Changed 5 years ago by
- Description modified (diff)
comment:5 Changed 5 years ago by
Magic string anti-pattern: https://en.wikipedia.org/wiki/Magic_string
comment:6 Changed 5 years ago by
- Commit changed from 5a0c4b0d9610c2eee9f8d25f929ae3047cb65df8 to 088fdefb1e91ae460f67fa31344f6cb834d0b117
Branch pushed to git repo; I updated commit sha1. New commits:
088fdef | Avoid magic string
|
comment:7 Changed 5 years ago by
Its still a magic string. If somebody were to change the magic value to
fastest_mirror = "http:// fastest mirror"
then the program doesn't work any more. Fragile and difficult to follow.
comment:8 Changed 5 years ago by
- Commit changed from 088fdefb1e91ae460f67fa31344f6cb834d0b117 to 92582788c1f3a01479f4414b466275120aa843ec
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
9258278 | Avoid magic string
|
comment:9 Changed 5 years ago by
Better like this?
comment:10 Changed 5 years ago by
- Status changed from needs_review to positive_review
The usual pattern is to throw exceptions that have the error message and catch them at the outermost level. Without relying on any local variable being in scope at the exception handler. But its an improvement, at least.
comment:11 Changed 5 years ago by
- Branch changed from u/jdemeyer/apply__18731_again to 92582788c1f3a01479f4414b466275120aa843ec
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Better error handling in sage-download-file