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: introduce null checks in manager #2816

Merged
merged 2 commits into from
May 4, 2023

Conversation

lanthoor
Copy link
Contributor

What this PR changes/adds

  • adds null check for policy in TransferProcessManager

Why it does that

To avoid redundant null checks from collaborators of the managers.

Further notes

N/A

Linked Issue(s)

Closes #2225

Checklist

  • added appropriate tests?
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • assigned appropriate label? (exclude from changelog with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and Etiquette for pull requests for details)

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

We are always happy to welcome new contributors ❤️ To make things easier for everyone, please make sure to follow our contribution guidelines, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.

@jimmarino jimmarino requested review from ndr-brt and jimmarino April 26, 2023 13:53
@lanthoor lanthoor temporarily deployed to Azure-dev April 29, 2023 14:42 — with GitHub Actions Inactive
@codecov-commenter
Copy link

codecov-commenter commented Apr 29, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.05 ⚠️

Comparison is base (357fe1a) 66.11% compared to head (a3b3562) 66.07%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2816      +/-   ##
==========================================
- Coverage   66.11%   66.07%   -0.05%     
==========================================
  Files         999      999              
  Lines       20200    20176      -24     
  Branches     1181     1181              
==========================================
- Hits        13355    13331      -24     
  Misses       6356     6356              
  Partials      489      489              
Impacted Files Coverage Δ
.../aws/s3/S3ConsumerResourceDefinitionGenerator.java 100.00% <ø> (ø)
...ectStorageConsumerResourceDefinitionGenerator.java 100.00% <ø> (ø)
...on/gcp/GcsConsumerResourceDefinitionGenerator.java 100.00% <ø> (ø)
.../impl/HttpProviderResourceDefinitionGenerator.java 100.00% <ø> (ø)
...th2/Oauth2ConsumerResourceDefinitionGenerator.java 100.00% <ø> (ø)
...th2/Oauth2ProviderResourceDefinitionGenerator.java 100.00% <ø> (ø)
...r/transfer/process/TransferProcessManagerImpl.java 88.16% <100.00%> (+0.10%) ⬆️

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

Copy link
Member

@ndr-brt ndr-brt left a comment

Choose a reason for hiding this comment

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

just a minor thing in the test, please resolve conflicts so we can merge

Signed-off-by: Lallu Anthoor <lallu.anthoor@sap.com>
@lanthoor lanthoor force-pushed the refactor/null-check-in-managers branch from 572e83e to 5a51473 Compare May 2, 2023 12:31
@lanthoor lanthoor force-pushed the refactor/null-check-in-managers branch from 5a51473 to a3b3562 Compare May 2, 2023 12:35
@lanthoor
Copy link
Contributor Author

lanthoor commented May 2, 2023

@ndr-brt @jimmarino I need a bit more time to cleanup the other manager. I will set the PR to "Ready for review" once done. Thank you for your reviews.

@lanthoor lanthoor marked this pull request as ready for review May 2, 2023 13:44
@lanthoor
Copy link
Contributor Author

lanthoor commented May 2, 2023

Could not find anything to refactor in ContractNegotiationManager implementations. Marking the PR ready for review.

@jimmarino
Copy link
Contributor

@lanthoor Please mark the comments as resolved and we can approve once done. Thx

@lanthoor
Copy link
Contributor Author

lanthoor commented May 3, 2023

@jimmarino done

@paullatzelsperger paullatzelsperger removed their request for review May 3, 2023 06:49
@jimmarino jimmarino self-requested a review May 3, 2023 07:20
@lanthoor lanthoor temporarily deployed to Azure-dev May 3, 2023 07:21 — with GitHub Actions Inactive
@lanthoor
Copy link
Contributor Author

lanthoor commented May 3, 2023

@jimmarino I do not have edit rights to the repo. Could you add the label refactoring and merge this PR, please?

@ndr-brt ndr-brt added the refactoring Cleaning up code and dependencies label May 4, 2023
@ndr-brt ndr-brt merged commit b0b552d into eclipse-edc:main May 4, 2023
@ndr-brt
Copy link
Member

ndr-brt commented May 4, 2023

@lanthoor thanks!

@lanthoor lanthoor deleted the refactor/null-check-in-managers branch May 4, 2023 14:58
majadlymhmd pushed a commit to FraunhoferISST/edc-connector that referenced this pull request May 10, 2023
* refactor: null check in TransferProcessManager

Signed-off-by: Lallu Anthoor <lallu.anthoor@sap.com>

* review comments

---------

Signed-off-by: Lallu Anthoor <lallu.anthoor@sap.com>
majadlymhmd pushed a commit to FraunhoferISST/edc-connector that referenced this pull request May 10, 2023
* refactor: null check in TransferProcessManager

Signed-off-by: Lallu Anthoor <lallu.anthoor@sap.com>

* review comments

---------

Signed-off-by: Lallu Anthoor <lallu.anthoor@sap.com>
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.

Leverage null-checks of not nullable objects to managers
4 participants