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

Rename EdcSetting to Setting #19

Closed
1 of 2 tasks
paullatzelsperger opened this issue Oct 5, 2022 · 6 comments · Fixed by #27, eclipse-edc/Connector#2097 or #47
Closed
1 of 2 tasks

Rename EdcSetting to Setting #19

paullatzelsperger opened this issue Oct 5, 2022 · 6 comments · Fixed by #27, eclipse-edc/Connector#2097 or #47
Assignees
Labels
enhancement New feature or request

Comments

@paullatzelsperger
Copy link
Member

Feature Request

If you are missing a feature or have an idea how to improve this project that should first be discussed, please feel free to open up a discussion.

Type of Issue

improvement

Checklist

  • assigned appropriate label?
  • Do NOT select a milestone or an assignee!
@paullatzelsperger paullatzelsperger added the enhancement New feature or request label Oct 5, 2022
@paullatzelsperger paullatzelsperger self-assigned this Oct 5, 2022
@ndr-brt
Copy link
Member

ndr-brt commented Oct 7, 2022

To make the transition softer we could introduce a new Setting annotation and deprecate EdcSetting for some time

@paullatzelsperger
Copy link
Member Author

paullatzelsperger commented Oct 10, 2022

To make the transition softer we could introduce a new Setting annotation and deprecate EdcSetting for some time

Here, deprecating seems like an awful lot of ceremony for a simple seach-and-replace job, because it is not as simple as just putting @Deprecated there: we'd also have to touch the annotation processor and the runtime metamodel, and then change it back later.

@paullatzelsperger
Copy link
Member Author

@ndr-brt I thought about it again and as it turns out, it may indeed make sense to deprecate it first, because otherwise all EDC builds would fail...
However, we should remove the @EdcSetting as soon as a corresponding refactoring PR in EDC is merged.

@ndr-brt
Copy link
Member

ndr-brt commented Oct 14, 2022

@ndr-brt I thought about it again and as it turns out, it may indeed make sense to deprecate it first, because otherwise all EDC builds would fail... However, we should remove the @EdcSetting as soon as a corresponding refactoring PR in EDC is merged.

Could we do that after Milestone 7 release? that would give some time to the downstream project to adapt, I think this is a good practice we should adopt

@paullatzelsperger
Copy link
Member Author

paullatzelsperger commented Oct 14, 2022

Could we do that after Milestone 7 release? that would give some time to the downstream project to adapt, I think this is a good practice we should adopt

We always said we do not guarantee stable APIs until version 1.0.0 and that breaking changes (API, SPI, etc.) are to be expected until such time. I do not want to get caught up in a situation where (in a pre-release version!) we need to keep track of what is deprecated, what is scheduled for removal etc. Here, the adoption should be trivial, because it's just a search-and-replace across the code base, since package declarations do not change.

@ndr-brt
Copy link
Member

ndr-brt commented Oct 17, 2022

Could we do that after Milestone 7 release? that would give some time to the downstream project to adapt, I think this is a good practice we should adopt

We always said we do not guarantee stable APIs until version 1.0.0 and that breaking changes (API, SPI, etc.) are to be expected until such time. I do not want to get caught up in a situation where (in a pre-release version!) we need to keep track of what is deprecated, what is scheduled for removal etc. Here, the adoption should be trivial, because it's just a search-and-replace across the code base, since package declarations do not change.

This will cost nothing to us and it will be an huge help for the community, we just need to wait 2 weeks to do it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment