Adds local filesystem support - #13
Conversation
|
Nice! Will start taking a look at this soon, maybe after the weekend. When it looks good and has tests, we can merge it in. |
| // The path to the directory where check files will be stored. | ||
| Dir string `json:"dir"` | ||
| // The URL corresponding to fs.Dir. | ||
| Url string `json:"url"` |
There was a problem hiding this comment.
This should be named URL according to https://github.com/golang/go/wiki/CodeReviewComments#initialisms.
|
|
||
| err = json.NewDecoder(f).Decode(&index) | ||
| return | ||
| } |
There was a problem hiding this comment.
As a matter of style, I would prefer not to have named returns here, and instead start the function with index := make(map[string]int64). Even though the function is small, I think the explicit returns are a little easier to follow, especially since you have to make the map anyway.
|
I think the CI tests are broken - @beyang, somebody want to take a look at that sometime? 😄 |
|
This PR is looking pretty good except for the html file change. Thanks for working on it, @emersion! |
|
All right, fixed all issues! The build is failing because there is no |
| } | ||
|
|
||
| // Add new file to index | ||
| index[name] = time.Now().UnixNano() |
There was a problem hiding this comment.
Shouldn't this be the same filename that you used above in os.Create? name is retrieved from GenerateFilename() but you use a potentially different time value here.
There was a problem hiding this comment.
The index contains both filenames and their timestamp, which is helpful because we don't need to parse filenames anymore. Since filenames are just informative, I though having a slightly different timestamp wouldn't matter too much...
There was a problem hiding this comment.
Oh, right, thanks for clarifying.
|
Awesome! Thank you @emersion!! |
|
Yay! 🎉 |
3 participants
Fixes #6