⚠️ Export scaffolding machinery #2082
Conversation
|
Skipping CI for Draft Pull Request. |
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
size/XXL
Adirio
force-pushed
the
export-scaffold-machinery
branch
from
f1eb924
to
723aa72
Compare
|
/test all |
k8s-ci-robot
added
the
needs-rebase
Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>
Adirio
force-pushed
the
export-scaffold-machinery
branch
from
723aa72
to
61dcae6
Compare
k8s-ci-robot
removed
the
needs-rebase
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
The change which was significant and was discussed in the meeting is: #2080. (which was approved and merged by Eric, who raised the concerns. )
This one is the only flag as breaking change because is moving things around and changing the names. However, the same feature options still in place. Also, it shows very small changes which means that shows fine we move forward here.
PS.: We also agree in the meetings/issues to export the machinery to remove this duplication from the consumer (SDK)
/lgtm
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Adirio, camilamacedo86 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| @@ -41,32 +49,96 @@ var options = imports.Options{ | |||
| } | |||
|
|
|||
| // Scaffold uses templates to scaffold new files | |||
| type Scaffold interface { | |||
There was a problem hiding this comment.
Why was this changed to a struct?
There was a problem hiding this comment.
Because we were exporting a Option that was a function on an unexported struct.
There was a problem hiding this comment.
You can do type ScaffoldOption func(*scaffold) since the predefined options have access to the internal type.
There was a problem hiding this comment.
I would prefer ^ but it's an unimportant design decision.
There was a problem hiding this comment.
That means that we are sporting ScaffoldOption which has a unexported parameter. I avoided this when able.
There was a problem hiding this comment.
That's a common practice. The set of options is fixed because Scaffold's fields are unexported anyway.
There was a problem hiding this comment.
Oh, yes, I know. The result is exactly the same. But there was not added value by the interface either. And this way we don't have a unexported type in pkg.go.dev.
It is technically a breaking change (⚠️ ) because some errors that were previously placed in
pkg/modelandpkg/model/filehave been moved topkg/machineryand no longer exposeNewXxxxxxErrorandIsXxxxxxxErrorfunctions, they expose themselves to useerrors.Asanderrors.Is. It also removes theNewUniversemethod and theWithXxxxxoptions are now part of theNewScaffoldmethod, instead of having to pass a universe to everyExecutecall.Closes: #2036
Blocked by: #2080