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

fix: update transfer-05-file-transfer-cloud sample #247

Merged
merged 3 commits into from
Apr 29, 2024

Conversation

bpetrovbg
Copy link
Contributor

What this PR changes/adds

updates the provider/consumer configurations, builds and README for transfer-05-file-transfer-cloud

Why it does that

Sample is out of date.

Linked Issue(s)

Closes Issue fetching Retrieve provider Contract Offers

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.'

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.

hi, thanks for contribution, please sign the ECA

@@ -36,6 +36,20 @@ dependencies {
implementation(libs.edc.dsp)

implementation(libs.edc.data.plane.selector.core)

//configuration from earlier examples
Copy link
Member

Choose a reason for hiding this comment

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

this is not "configuration" but these are dependencies, please update the comment accordingly (or removing it altogether, as it is not really necessary). same for the other build file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are right, I just did a minor cleanup 😉

@ndr-brt ndr-brt added the bug Something isn't working label Apr 26, 2024
@ndr-brt
Copy link
Member

ndr-brt commented Apr 29, 2024

@bpetrovbg please fix the PR name according to the convention https://eclipse-edc.github.io/docs/#/documentation/CONTRIBUTING
when the PR is ready please re-request my review so I can eventually approve and merge

@bpetrovbg bpetrovbg changed the title Update transfer-05-file-transfer-cloud sample fix: update transfer-05-file-transfer-cloud sample Apr 29, 2024
@bpetrovbg bpetrovbg changed the title fix: update transfer-05-file-transfer-cloud sample fix: update transfer-05-file-transfer-cloud connector configurations, builds and README Apr 29, 2024
@bpetrovbg bpetrovbg requested a review from ndr-brt April 29, 2024 08:15
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.

@bpetrovbg bpetrovbg changed the title fix: update transfer-05-file-transfer-cloud connector configurations, builds and README fix: update transfer-05-file-transfer-cloud connector sample Apr 29, 2024
@bpetrovbg bpetrovbg changed the title fix: update transfer-05-file-transfer-cloud connector sample fix: update transfer-05-file-transfer-cloud sample Apr 29, 2024
@bpetrovbg
Copy link
Contributor Author

bpetrovbg commented Apr 29, 2024

@bpetrovbg the PR name is still not valid: https://github.com/eclipse-edc/Samples/actions/runs/8875401453/job/24366181117?pr=247

yes, the name was probably too long... 😀

@ndr-brt ndr-brt merged commit 85fc9a4 into eclipse-edc:main Apr 29, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue fetching Retrieve provider Contract Offers
2 participants