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 marshaling policy #4844

Closed

Conversation

ChMThiel
Copy link

@ChMThiel ChMThiel commented Feb 25, 2025

description:
Consider property 'profile' when marshaling a 'Policy'-Object to json.

fixes #4843

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 contributors manual, check if you have already signed the ECA, and relate this pull request to an existing issue or discussion.

@paullatzelsperger
Copy link
Member

paullatzelsperger commented Feb 25, 2025

@ChMThiel this PR violates multiple rules in our PR Etiquette:

  • issue not triaged
  • you don't have a committer as sponsor
  • PR title is not conformant with our rules
  • PR description is missing
  • code style is insufficient. in fact, there is a violation in almost every line of the changeset. use static imports, don't waste one line per method argument, don't add useless comments like //when and //then, test method name...

In addition, you need to sign the ECA.

you may think it's a minor issue, but unless you follow proper procedure and respect our rules and guidelines, this PR is going to get closed.

@ChMThiel ChMThiel changed the title Fix/marshal policy issue https://github.com/eclipse-edc/Connector/issues/4843 Fix marshaling policy Feb 25, 2025
@ChMThiel
Copy link
Author

@ChMThiel this PR violates multiple rules in our PR Etiquette:

* issue not triaged

* you don't have a committer as sponsor

* PR title is not conformant with our rules

* PR description is missing

* code style is insufficient. in fact, there is a violation in almost every line of the changeset. use static imports, don't waste one line per method argument, don't add useless comments like `//when` and `//then`, test method name...

In addition, you need to sign the ECA.

you may think it's a minor issue, but unless you follow proper procedure and respect our rules and guidelines, this PR is going to get closed.

I'm sorry if i don't meet your expectations.
I changed the title of the PR
Regarding the code style: I took the existing test shouldMapPolicyTypeToOdrlType as a pattern. If there is a problem with the behavior-driven-development style of the test, i can remove the given/when/then and rename the test-method to any style you like - i found no special rules about that...
I have no sponsors etc. perhaaps you can help me with that.
All I wanted to show with the (premature) PR is how simple the fix would be.

@paullatzelsperger
Copy link
Member

please take a look at the document I linked. proper formal procedure is as important as code style.
My recommendation would be to close this PR and start with a clean slate, await issue triage etc.

@ChMThiel ChMThiel closed this Feb 25, 2025
@ChMThiel
Copy link
Author

please take a look at the document I linked. proper formal procedure is as important as code style. My recommendation would be to close this PR and start with a clean slate, await issue triage etc.

FYI the links in the pr-etiquette page: coding style, ./architectural patterns, are dead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants