Add a function of multiple deletion to Cloud Store./Motivation

CommentFileSizeAuthor
#18 3194623-19.patch26.62 KBMasatoTakada
#17 3194623-17.patch25.89 KByas

Issue fork cloud-3194623

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

MasatoTakada created an issue. See original summary.

MasatoTakada opened merge request !121

MasatoTakada’s picture

MasatoTakada commented
Status: Active » Needs review
MasatoTakada’s picture

MasatoTakada commented

@yas

I uploaded my patch. Please review it.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@masatotakada

Thank you for the refactoring. Please see my above comments. Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@masatotakada

Thank you for the update. I reviewed the patch and posted my comment.

MasatoTakada closed merge request !121

MasatoTakada opened merge request !125

MasatoTakada closed merge request !125

MasatoTakada reopened merge request !125

MasatoTakada’s picture

MasatoTakada commented
Status: Needs work » Needs review
MasatoTakada’s picture

MasatoTakada commented

@yas
I just pushed my patch. Please check it.

yas credited Masami.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Needs work

@masatotakada

Thank you for the update. Please see my above comment. Thanks.

MasatoTakada’s picture

MasatoTakada commented
Status: Needs work » Needs review

@yas

I removed while loop from test code. Please check it.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented

@masatotakada

Thank you for the update.

@masami

What do you think? Do you have any idea to refactor for the following if-statement? If you are ok as it is, it should be fine.

https://git.drupalcode.org/project/cloud/-/merge_requests/125/diffs#24d0...

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Title: Add a function of multiple deletion to Cloud Store. » Add a function of multiple deletion to CloudStore
StatusFileSize
new 3194623-17.patch25.89 KB

@masatotakada

Thank you for the update. This looks good now and even more clearer than before.

@masami

What do you think?

MasatoTakada’s picture

MasatoTakada commented
StatusFileSize
new 3194623-19.patch26.62 KB

@yas

I uploaded the patch to resolve conflict.
Please check it again.

Masato

masami’s picture

masami commented

@yas
@masatotakada
The patch looks good to me.

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Needs review » Reviewed & tested by the community

@masami

Thank you for your review. I tested the patch and it looks good to me. I'll merge the patch to 3.x and close this issue as Fixed.

  • yas committed 02a7712 on 3.x authored by MasatoTakada 
    Issue #3194623 by MasatoTakada, yas, Masami: Add a function of multiple...

yas closed merge request !125

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.