-
Notifications
You must be signed in to change notification settings - Fork 441
feat(upload): Add HTTP method selection for upload #2991
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
feat(upload): Add HTTP method selection for upload #2991
Conversation
ca66499 to
8208981
Compare
|
Thanks for the PR! The main maintainers are out of office (or will be in the next 1-2 days) for the next 2-3 weeks so this may get delayed quite a bit. P.S. Please revert the changes in Cargo.toml and package.json. Versioning will be handled in CI. |
0331789 to
e7e958a
Compare
|
Thanks @FabianLars - Changes made, good to know versioning is handled by CI. |
Package Changes Through d2f099eThere are 6 changes which include log with minor, log-js with minor, updater with minor, updater-js with minor, upload with minor, upload-js with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
what do you think about using https://docs.rs/http/1.3.1/http/method/struct.Method.html instead of a custom enum? It's already accessible via |
@FabianLars Good question. Had a look and while we could use With this in mind it seems simpler/safer to just use a custom enum type in Rust. What do you think? |
7d5594f to
c634107
Compare
c634107 to
466c64c
Compare
|
@FabianLars Rebased - All tests are passing locally. |
|
Very helpful |
I don't think there's much of a difference, but when i wrote my comment i thought the plugin had a rust api but since that's not true, i'm fine with your approach. p.s. re-run |
466c64c to
bea72f2
Compare
Thanks @FabianLars. Just fixed CI build per your comments. |
FabianLars
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks :)
Resolves:
#2847
Key Changes