shopware icon indicating copy to clipboard operation
shopware copied to clipboard

feat: add interface to support copy in batches

Open tinect opened this issue 1 year ago • 12 comments

1. Why is this change necessary?

As a manufacturer of a flysystem adapter I want to be able to use the writeBatch-possibilities with a dedicated Interface until flysystem supports it out-of-the-box.

2. What does this change do, exactly?

  • change the naming of CopyBatch to WriteBatch
  • added dedicated WriteBatch-Adapter
  • added Interface

3. Describe each step to reproduce the issue or behaviour.

4. Please link to the relevant issues (if any).

5. Checklist

  • [x] I have rebased my changes to remove merge conflicts
  • [x] I have written tests and verified that they fail without my change
  • [x] I have created a changelog file with all necessary information about my changes
  • [ ] I have written or adjusted the documentation according to my changes
  • [ ] This change has comments for package types, values, functions, and non-obvious lines of code
  • [x] I have read the contribution requirements and fulfill them.

tinect avatar Oct 04 '24 21:10 tinect

Warnings
:warning: Please be kind and add unit tests for your new code in these files:
src/Core/Framework/Adapter/Filesystem/Adapter/AsyncAwsS3WriteBatchAdapter.php
src/Core/Framework/Adapter/Filesystem/Plugin/WriteBatchInterface.php
If you are sure everything is fine with your changes, you can resolve this warning.
You can run `composer make:coverage` to generate dummy unit tests for files that are not covered

github-actions[bot] avatar Oct 04 '24 21:10 github-actions[bot]

hey @tinect what's the state here? Are you still working on it?

AydinHassan avatar Oct 09 '24 15:10 AydinHassan

hey @tinect what's the state here? Are you still working on it?

Oh yes. I need to test it again and discuss the test case with shyim. I will ping you if I am ready.

tinect avatar Oct 09 '24 16:10 tinect

Got it 🫡

AydinHassan avatar Oct 09 '24 18:10 AydinHassan

@AydinHassan It is ready :-)

tinect avatar Oct 11 '24 05:10 tinect

@tinect what's the reason for the rename? Is it worth it?

AydinHassan avatar Oct 11 '24 06:10 AydinHassan

@tinect what's the reason for the rename? Is it worth it?

It is more style, while it has less to do with "copy", but with "write" regarding the filesystem/flysystem.

tinect avatar Oct 11 '24 07:10 tinect

okay thanks. I am off for few days now, and if no one gets to it before, I will organise an internal review end of next week and then we go from there. Hope that's okay!

AydinHassan avatar Oct 11 '24 07:10 AydinHassan

Thank you, have nice days! :-)

tinect avatar Oct 11 '24 07:10 tinect

hey @tinect - we had a short discussion and the team is in agreement that the write -> copy change is not really worth the deprecations, could you update the PR please?

AydinHassan avatar Oct 16 '24 07:10 AydinHassan

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 43.58%. Comparing base (21cee05) to head (a01f0fd). Report is 9077 commits behind head on trunk.

Additional details and impacted files
@@             Coverage Diff             @@
##            trunk    #4976       +/-   ##
===========================================
- Coverage   65.46%   43.58%   -21.89%     
===========================================
  Files        3677     3072      -605     
  Lines       82789    87180     +4391     
===========================================
- Hits        54201    37996    -16205     
- Misses      28588    49184    +20596     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Oct 20 '24 18:10 codecov[bot]

@AydinHassan ready :-) thank you.

tinect avatar Oct 20 '24 18:10 tinect

thanks @tinect I will import it now :)

AydinHassan avatar Oct 21 '24 11:10 AydinHassan

and it's merged 🎉 Thanks for working with us on this @tinect !

AydinHassan avatar Oct 21 '24 13:10 AydinHassan