Feature/103 move blueprint structs to blueprint-lib #104
Conversation
…traction" This reverts commit 0e10672.
This commit moves all adapter-related blueprint struct code into the new blueprint-lib. The changes that go beyond simple extraction (like conversion and deepcopy) were necessary to make the code work like before. This also uncovered the fact that the blueprintcr package has relation- ships towards the serializer adapter which seemed weird. Probably another disconnect is necessary.
The previously mentioned make target didn't work for me because no target with that name was found. `make generate-deepcopy` did work out though.
There was a problem hiding this comment.
Your extraction of the serializer package is easy to understand and should work this way for now but it leads to some problems in the long run. It is very unfortunate that the deadline for your project is so close, so that we have no real choice here than merging it.
Under normal conditions, we would not accept this as we already have a better but more complex plan to extract the blueprint CRD together with its generation process and its Rest-Client. I think this PR does not help us or makes it even more complicated to achieve this.
Out of the scope of this PR, i think it is also not a good idea that you put the old blueprint and parts of the new blueprint-CR into the same lib. They should be in separate libs as part of our plans was to minimize the dependencies to the old code. This way we maybe can just change your lib when we start with our planned refactoring.
CHANGELOG.md
Outdated
| - to parse different blueprint versions. | ||
|
|
||
| ### Fixed | ||
| - Fix autogeneration boilerplate text to match with the current make target `` |
There was a problem hiding this comment.
| - Fix autogeneration boilerplate text to match with the current make target `` | |
| - Fix autogeneration boilerplate text to match with the current make target `make generate` |
is that what you mean?
There was a problem hiding this comment.
I did deffo miss hier the actual target. But it was make generate-deepcopy. I will fix this.
|
Why is there a security label at the PR? |
Because an update of an x/stdlib package removed a high CVE |
I agree. With the whole lot of CR stuff and even more about to be extracted this makes sense. I will move the blueprint operator specific code into a new repository and will update the code after moving it there |
|
There was a problem hiding this comment.
The new compromise with the separate library is way better. We can use it for further refactorings in this form. Like I stated in the last review, your changes are very straight forward. I will approve the changes if you fix the small mistake in the changelog :)
Sorry for the harsh words in the last review!
CHANGELOG.md
Outdated
|
|
||
| ### Fixed | ||
| - Fix autogeneration boilerplate text to match with the current make target `` | ||
| - Fix autogeneration boilerplate text to match with the current make target `generator-deepcopy` |
There was a problem hiding this comment.
| - Fix autogeneration boilerplate text to match with the current make target `generator-deepcopy` | |
| - Fix autogeneration boilerplate text to match with the current make target `generate-deepcopy` |
|
This PR resolves #103 by extracting blueprint structs and moving it into the blueprint-lib. See also cloudogu/blueprint-lib#1
This PR also resolves slight fixes as can be seen in the changelog.