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

fix: remove unnecessary validation #467

Merged
merged 1 commit into from
Dec 29, 2021
Merged

Conversation

mitchell-codecov
Copy link
Contributor

@mitchell-codecov mitchell-codecov commented Oct 26, 2021

A future PR could

  • use the ECMAScript URL class constructor to validate URLs passed by the user
  • modify the upload-related functions to accept ECMAScript URL objects in place of their current string parameters

Fixes #452

Verified

This commit was signed with the committer’s verified signature.
drazisil-codecov Joe Becher
@mitchell-codecov mitchell-codecov requested a review from a team as a code owner October 26, 2021 19:18
@codecov
Copy link

codecov bot commented Oct 26, 2021

Codecov Report

Merging #467 (66337ed) into master (2cf31fb) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #467      +/-   ##
==========================================
- Coverage   92.91%   92.90%   -0.01%     
==========================================
  Files          27       27              
  Lines         875      874       -1     
  Branches      172      172              
==========================================
- Hits          813      812       -1     
  Misses         27       27              
  Partials       35       35              
Flag Coverage Δ
alpine 92.90% <0.00%> (-0.01%) ⬇️
alpine-without-git 92.90% <0.00%> (-0.01%) ⬇️
linux 92.90% <0.00%> (-0.01%) ⬇️
linux-without-git 92.90% <0.00%> (-0.01%) ⬇️
macos 92.90% <0.00%> (-0.01%) ⬇️
macos-without-git 92.90% <0.00%> (-0.01%) ⬇️
windows 92.90% <0.00%> (-0.01%) ⬇️
windows-without-git 92.90% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/index.ts 81.25% <0.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2cf31fb...66337ed. Read the comment docs.

@mitchell-codecov mitchell-codecov changed the title fix: remove unnecessary validation (DO NOT MERGE) fix: remove unnecessary validation Oct 26, 2021
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2021

This pull request introduces 1 alert when merging 66337ed into 2cf31fb - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@drazisil-codecov
Copy link
Contributor

@mitchell-codecov status check :)

@mitchell-codecov
Copy link
Contributor Author

This was marked "DO NOT MERGE" due to a minor UX concern. If a truly invalid URL is passed by the user, a fairly low-level error will be shown rather than one directly corresponding to the would-be invalid input.

@mitchell-codecov mitchell-codecov changed the title (DO NOT MERGE) fix: remove unnecessary validation fix: remove unnecessary validation Dec 29, 2021
@mitchell-codecov mitchell-codecov merged commit 4110ea7 into master Dec 29, 2021
@mitchell-codecov mitchell-codecov deleted the fix/url-validation branch December 29, 2021 16:51
This was referenced Jan 18, 2022
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.

Uploader does not allow all possible values to be passed as override urls.
2 participants