Update files as part of the Scaffold.Execute #1375

Adirio · 2020-02-17T09:15:53Z

Description

Update already scaffolded files inside Scaffold.Execute following the same flow used for new files (injecting fields, validating, creating a model, piping to plugins and persisting it to disk).

Coverage reports 72.1% in pkg/scaffold/internal/filesystem (the remaining 27.9% are calls to the filesystem that cannot be further tested) and 93.8% in pkg/scaffold/internal/machinery, aiming to fulfill #1342.

Motivation

File updates (i.e., inserting code fragments to already scaffolded files) was not needed for v1. When v2 came, the feature was needed and it was added outside of the main Scaffold.Execute. Providing Scaffold.Execute the capability of inserting those code fragments achieves a better work flow.

Implementation details

This PR is made of 3 commits. The first two were submitted as separate PR previously:

Template mixins #1359
File interface #1227
I suggest reviewing them commit by commit (GitHub offers the possibility of showing only changes in one commit) as it affects more than 3500 LoC.

Regarding to the last commit, the main changes are found in pkg/scaffold/internal/machinery/scaffold.go and the interfaces in pkg/model/file/interfaces.go.

This PR is scoped under #1218
Closes #1226
Closes #1227
Closes #1358
Closes #1359

k8s-ci-robot · 2020-02-17T09:16:01Z

Hi @Adirio. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>

DirectXMan12

few minor comments inline. gonna give @mengqiy the final signoff

pkg/model/file/template.go

pkg/model/file/marker.go

pkg/scaffold/internal/filesystem/errors.go

pkg/scaffold/internal/templates/v2/controller/controller_suitetest.go

mengqiy · 2020-02-20T22:42:46Z

Closes #1226
Closes #1227
Closes #1358
Closes #1359

2 of them are PRs. Why?

EDIT:
nvm
it seems we need to merge those 2 PRs first.

Adirio · 2020-02-20T23:08:02Z

2 of them are PRs. Why?

The two first commits in this PR are those PRs. Merging this would be like merging the three.

mengqiy · 2020-02-20T23:52:37Z

pkg/model/file/marker.go

+const prefix = "+kubebuilder:scaffold:"
+
+var commentsByExt = map[string]string{
+	// TODO(v3): machine-readable comments should not have spaces by Go convention. However,


Can you create an issue to track everything related to TODO(v3)?

#1382, #1383, and #1384 raised. Could you add them to the corresponding milestone?

mengqiy

/lgtm
/approve
we can address the nits in a followup PR.

k8s-ci-robot · 2020-02-20T23:53:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Adirio, mengqiy

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:

~~OWNERS~~ [mengqiy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 17, 2020

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 17, 2020

k8s-ci-robot requested review from joelanford and mengqiy February 17, 2020 09:16

Adirio force-pushed the scaffold-enhancement/file-update branch 3 times, most recently from 2bc3be0 to 5114a80 Compare February 17, 2020 11:51

Adirio added 2 commits February 20, 2020 11:08

Template mixins

435f946

Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>

Template interface (phase 1)

e80f0a1

Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>

Adirio force-pushed the scaffold-enhancement/file-update branch from 5114a80 to 86f27f7 Compare February 20, 2020 10:09

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 20, 2020

Adirio force-pushed the scaffold-enhancement/file-update branch 8 times, most recently from 607c49f to a4516bf Compare February 20, 2020 13:55

Template interface (phase 2) including insert code fragments logic

6f191bf

Signed-off-by: Adrian Orive <adrian.orive.oneca@gmail.com>

Adirio force-pushed the scaffold-enhancement/file-update branch from a4516bf to 6f191bf Compare February 20, 2020 14:07

Adirio changed the title ~~[WIP] Update files as part of the Scaffold.Execute~~ Update files as part of the Scaffold.Execute Feb 20, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2020

Adirio mentioned this pull request Feb 20, 2020

[Umbrella issue] Scaffolding enhancements #1218

Closed

DirectXMan12 reviewed Feb 20, 2020

View reviewed changes

Adirio mentioned this pull request Feb 20, 2020

Feature Request: Plugins Phase 2 #1378

Closed

mengqiy added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Feb 20, 2020

mengqiy reviewed Feb 20, 2020

View reviewed changes

k8s-ci-robot assigned mengqiy Feb 20, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 20, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 20, 2020

k8s-ci-robot merged commit c5fd438 into kubernetes-sigs:master Feb 20, 2020

Adirio deleted the scaffold-enhancement/file-update branch February 21, 2020 07:27

This was referenced Feb 21, 2020

Remove outdated TODO #1374

Closed

Implement Wrapper interface for errors #1349

Merged

NITs raised in 1375 #1388

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update files as part of the Scaffold.Execute #1375

Update files as part of the Scaffold.Execute #1375

Adirio commented Feb 17, 2020 •

edited

Loading

k8s-ci-robot commented Feb 17, 2020

DirectXMan12 left a comment

mengqiy commented Feb 20, 2020 •

edited

Loading

Adirio commented Feb 20, 2020

mengqiy Feb 20, 2020

Adirio Feb 21, 2020

mengqiy left a comment

k8s-ci-robot commented Feb 20, 2020

Update files as part of the Scaffold.Execute #1375

Update files as part of the Scaffold.Execute #1375

Conversation

Adirio commented Feb 17, 2020 • edited Loading

Description

Motivation

Implementation details

k8s-ci-robot commented Feb 17, 2020

DirectXMan12 left a comment

Choose a reason for hiding this comment

mengqiy commented Feb 20, 2020 • edited Loading

Adirio commented Feb 20, 2020

mengqiy Feb 20, 2020

Choose a reason for hiding this comment

Adirio Feb 21, 2020

Choose a reason for hiding this comment

mengqiy left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Feb 20, 2020

Adirio commented Feb 17, 2020 •

edited

Loading

mengqiy commented Feb 20, 2020 •

edited

Loading