feat: configuration for file-system config types for plugins and internal plugins#390
Conversation
500d764 to
b6a310d
Compare
b6a310d to
0a45f1b
Compare
9c425ca to
b8eed0f
Compare
jakobmoellerdev
left a comment
There was a problem hiding this comment.
The general design of the changes is nice! I am wondering about some nits and if you can really change all of these modules at once. I.e. youre changing the CLI but are also changing the OCI binding. The changes in OCI won't propagate to CLI if you do not separate these PRs out or create follow-ups. Whats your plan here?
|
@jakobmoellerdev My plan was that these modules are pretty much separated right now. Yes, the configuration doesn't exist yet through the CLI but I thought the WithConfiguration part can be introduced separately already I guess in this PR. But I can also separate that out. |
|
As i explained above, the modules are not separated. The only reason you compile is because you do not upgrade the modules yet. But for example, in OCI you have usages of OpenCTF that would be affected by your changes and that would anyhow require a second PR |
|
Correct. That's why it's safe to do so and then do an upgrade and apply all the things. :) That's what I meant. That said, I can do all this in separate PRs after each other if you prefer it that way. |
82db725 to
28669d8
Compare
684421e to
333b356
Compare
jakobmoellerdev
left a comment
There was a problem hiding this comment.
just two things I noticed during the final run, other than that great job 🥳
…rnal plugins Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> On-behalf-of: @SAP gergely.brautigam@sap.com
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
b24a43c to
7fe7c95
Compare
What this PR does / why we need it
Which issue(s) this PR fixes