Closed teovin closed 3 months ago
Attention: Patch coverage is 13.51351%
with 32 lines
in your changes are missing coverage. Please review.
Project coverage is 70.33%. Comparing base (
fbe925c
) to head (41b6638
). Report is 51 commits behind head on develop.:exclamation: Current head 41b6638 differs from pull request most recent head 5154e4c. Consider uploading reports for the commit 5154e4c to get more accurate results
Files | Patch % | Lines |
---|---|---|
perma_web/perma/celery_tasks.py | 13.88% | 31 Missing :warning: |
perma_web/tasks/dev.py | 0.00% | 1 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@teovin LGTM 🏄 ! I made few minor suggestions.
CCing @rebeccacremona for a second look, especially around Perma.cc codebase conventions.
For the required ts field of all urls, I used the Link model's creation_timestamp field, let me know if that is ok. I can also add any additional attributes (id, text and size) to the urls as we wish.
- As long as
ts
is a valid RFC3339 value, it should be fine. Have you had a chance to check if the date you passed shows up in the "Pages" menu of replayweb.page?- I don't believe the extra parameters are needed in that context 😄
Thank you!
@matteocargnelutti Thank you! Yep, they show up in replayweb.page.
This pull request was deployed and Sentry observed the following issues:
perma.celery_tasks.convert_warc_to_wacz
View IssueDid you find this useful? React with a 👍 or 👎
ENG-786
creation_timestamp
field, let me know if that is ok. I can also add any additional attributes (id, text and size) to the urls as we wish.