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

refactor: get rid of managedResource #3297

Merged

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented Jul 13, 2023

What this PR changes/adds

removed the managedResource field on DataRequest and all the accessors because it had no more meaning in the current situation

Why it does that

refactoring

Further notes

List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.

Linked Issue(s)

Closes #3292

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@ndr-brt ndr-brt added the refactoring Cleaning up code and dependencies label Jul 13, 2023
@ndr-brt ndr-brt requested a review from jimmarino July 13, 2023 10:19
Copy link
Member

@paullatzelsperger paullatzelsperger left a comment

Choose a reason for hiding this comment

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

probably also wanna update the SQL schema: extensions/control-plane/store/sql/transfer-process-store-sql/docs/schema.sql#L68

@ndr-brt ndr-brt force-pushed the 3292-get-rid-managed-resources branch from 1140c50 to 6c4c12d Compare July 13, 2023 11:43
@ndr-brt ndr-brt force-pushed the 3292-get-rid-managed-resources branch from 6c4c12d to ab3c3dc Compare July 13, 2023 11:49
@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2023

Codecov Report

Patch coverage: 41.66% and project coverage change: -54.46 ⚠️

Comparison is base (034258e) 72.20% compared to head (146dca0) 17.74%.

❗ Current head 146dca0 differs from pull request most recent head e23f408. Consider uploading reports for the commit e23f408 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3297       +/-   ##
===========================================
- Coverage   72.20%   17.74%   -54.46%     
===========================================
  Files         837      837               
  Lines       16867    16832       -35     
  Branches      951      948        -3     
===========================================
- Hits        12178     2987     -9191     
- Misses       4296    13767     +9471     
+ Partials      393       78      -315     
Impacted Files Coverage Δ
...r/transfer/process/TransferProcessManagerImpl.java 22.92% <0.00%> (-67.10%) ⬇️
...nsfer/provision/ResourceManifestGeneratorImpl.java 0.00% <ø> (-100.00%) ⬇️
...ment/transferprocess/model/TransferRequestDto.java 23.07% <ø> (-76.93%) ⬇️
...orm/JsonObjectToTransferRequestDtoTransformer.java 0.00% <ø> (-76.93%) ⬇️
...ransferRequestDtoToTransferRequestTransformer.java 0.00% <ø> (-100.00%) ⬇️
...s/store/schema/TransferProcessStoreStatements.java 96.29% <ø> (-0.14%) ⬇️
...cess/store/schema/postgres/DataRequestMapping.java 100.00% <ø> (ø)
.../edc/connector/transfer/spi/types/DataRequest.java 0.00% <ø> (-23.92%) ⬇️
...nector/transfer/spi/types/ProvisionedResource.java 0.00% <ø> (-87.50%) ⬇️
...tor/transfer/spi/types/ProvisionedResourceSet.java 0.00% <ø> (-61.91%) ⬇️
... and 3 more

... and 537 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ndr-brt ndr-brt force-pushed the 3292-get-rid-managed-resources branch from 146dca0 to e23f408 Compare July 13, 2023 13:07
@ndr-brt ndr-brt merged commit 6ee1137 into eclipse-edc:main Jul 13, 2023
@ndr-brt ndr-brt deleted the 3292-get-rid-managed-resources branch July 13, 2023 13:49
bjungs pushed a commit to imec-int/edc that referenced this pull request Aug 18, 2023
* refactor: get rid of managedResource

* PR remarks

* new dependencies file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: get rid of the managedResource field
4 participants