Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dompurify add vendor type #94

Merged
merged 2 commits into from Aug 5, 2022
Merged

Dompurify add vendor type #94

merged 2 commits into from Aug 5, 2022

Conversation

iisa
Copy link
Contributor

@iisa iisa commented Aug 5, 2022

problem: offshoot is not properly building bc the vendor package dompurify has an implicit type.

solution: make shim type file to declare it

acceptance: offshoot cleanly builds

note: 0.2.14-a1

@github-actions
Copy link

github-actions bot commented Aug 5, 2022

PR Preview Action v1.1.1
Preview removed because the pull request was closed.
2022-08-05 21:42 UTC

@codecov
Copy link

codecov bot commented Aug 5, 2022

Codecov Report

Merging #94 (f6ba5d7) into main (3831da5) will decrease coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #94      +/-   ##
==========================================
- Coverage   76.58%   76.46%   -0.12%     
==========================================
  Files          61       61              
  Lines        6726     6726              
  Branches      199      198       -1     
==========================================
- Hits         5151     5143       -8     
- Misses       1567     1575       +8     
  Partials        8        8              
Impacted Files Coverage Δ
src/tiles/item-image.ts 88.39% <0.00%> (-7.15%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mmcnl
Copy link

mmcnl commented Aug 5, 2022

@iisa I don't understand the backstory on this but LGTM. I do notice that the codecov check failed (https://github.com/internetarchive/iaux-collection-browser/pull/94/checks?check_run_id=7698782961)

@iisa
Copy link
Contributor Author

iisa commented Aug 5, 2022

hi @mmcnl thanks for the quick CR.

re: coverage drop, I guess adding type context opened up lines to check? here's the diff - nothing I touched https://app.codecov.io/gh/internetarchive/iaux-collection-browser/compare/94/changes

re backstory:
offshoot apparently type checks all ts files that it uses to build and it caught Dompurify as any - blocking @dualcnhq 's latest offshoot MR https://git.archive.org/www/offshoot/-/merge_requests/237
image

so, this is direct fix for it. and our work around for this

I will take the test hit bc it is out of scope for my debug time and we can work to make up for it in the next rounds. hope that's ok with you.

@mmcnl
Copy link

mmcnl commented Aug 5, 2022

@iisa sure, totally makes sense, thanks for filling me in on your process. I just would want us to not get in a long-term habit of ignoring codecov (or other) CI checks

@iisa iisa merged commit ed9dc28 into main Aug 5, 2022
@iisa iisa deleted the dompurify-add-type branch August 5, 2022 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants