Fix the reported modification time of reports. #16
Conversation
068cbe1 to
35b9fdb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
==========================================
- Coverage 99.44% 98.42% -1.02%
==========================================
Files 8 8
Lines 179 254 +75
==========================================
+ Hits 178 250 +72
- Misses 1 4 +3 ☔ View full report in Codecov by Sentry. |
8b8b699 to
327ca1c
Compare
The `/report/list` endpoint uses the `git_ls` function to enumerate reports, and supposedly includes the modification time is the response. Unfortunately the modification time returned by `git_ls` is pretty useless, and actually refers to the modification times in the local worktree, which has nothing to do with commit times. The proper way to do this is to identify the last commit to have touched the file, and then get the time of that commit. Unfortunately gert does not have an easy way to list commits on just one file (and in fact neither does libgit2), so we fallback to using the `git rev-list` command. Additionally I have re-written the code that was used to compare reports against the default branch to use Git plumbing commands instead of the high-level porcelain ones.
c5e5c06 to
d6557fc
Compare
The `as.POSIXct.numeric` function was relaxed a bit in R 4.3.0 and made its "origin" parameter optional, and we need that feature. For some reason the dependency installation was failing. Using `pak` seems to fix it. pak is generally a bit faster and more reliable anyway.
d6557fc to
6b3e83a
Compare
499ef62 to
2d078a4
Compare
| @@ -1,27 +1,15 @@ | |||
| FROM rocker/r-ver:4.1 | |||
| FROM rocker/r-ver:4.4 | |||
There was a problem hiding this comment.
maybe code has changed since but we intentionally pinned to 4.1 because of url params decoding issues in this PR #7 (comment), i dont remember the exact details but the url params parser changed to expect a different arg in base R with this change and that was breaking stuff when the image was built with 4.4. Just something to potentially be wary of if the docker image fails for a random reason
There was a problem hiding this comment.
Interesting. Thanks for the context.
I did try to use this new image with Packit yesterday and it seemed to work fine. Will keep this in mind though.
The
/report/listendpoint uses thegit_lsfunction to enumerate reports, and supposedly includes the modification time is the response.Unfortunately the modification time returned by
git_lsis pretty useless, and actually refers to the modification times in the local worktree, which has nothing to do with commit times.The proper way to do this is to identify the last commit to have touched the file, and then get the time of that commit. Unfortunately gert does not have an easy way to list commits on just one file (and in fact neither does libgit2), so we fallback to using the
git rev-listcommand.Additionally I have re-written the code that was used to compare reports against the default branch to use Git plumbing commands instead of the high-level porcelain ones.