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

feat: reupload #323

Merged
merged 2 commits into from
May 26, 2021
Merged

feat: reupload #323

merged 2 commits into from
May 26, 2021

Conversation

AuHau
Copy link
Contributor

@AuHau AuHau commented May 21, 2021

Closes #313

@AuHau AuHau requested a review from a team May 21, 2021 08:12
@AuHau AuHau requested a review from nugaon as a code owner May 21, 2021 08:12
Copy link
Member

@nugaon nugaon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good, regarding to the axiosOptions on module level we can provide this additionally but I also don't see real any use-case now for that now.
for the Bee issue I'd rather propose using 422 http error, as the syntax and the request are in great format and understandable by the "server", but the business-logic does not let reupload an unpinned content.

@AuHau
Copy link
Contributor Author

AuHau commented May 25, 2021

The code looks good, regarding to the axiosOptions on module level we can provide this additionally but I also don't see real any use-case now for that now.

@nugaon you mean for this reupload endpoint or generally in Bee JS?

for the Bee issue I'd rather propose using 422 http error, as the syntax and the request are in great format and understandable by the "server", but the business-logic does not let reupload an unpinned content.

Hmm not sure how it is related to this? I would suggest to comment in the linked issue as this is up to the Bee team to decide

@nugaon
Copy link
Member

nugaon commented May 25, 2021

@nugaon you mean for this reupload endpoint or generally in Bee JS?

I meant I don't see the use-case for this reupload endpoint regarding to the axios options. anyway it can be unified as in our other modules we do the same, it doesn't pollute the code.

@nugaon
Copy link
Member

nugaon commented May 25, 2021

Hmm not sure how it is related to this? I would suggest to comment in the linked issue as this is up to the Bee team to decide

it was commented in your code in this PR.

@AuHau AuHau requested review from agazso and nugaon May 26, 2021 06:48
@AuHau AuHau merged commit 3a256f8 into master May 26, 2021
@AuHau AuHau deleted the feat/reupload branch May 26, 2021 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reupload of data
3 participants