Skip to content

Commit a6a3990

Browse files
authored
refactor: inline fields from Cfg (#2159)
Remove the Cfg field from BuildRequest, ConfigureRequest, GenerateRequest, and ReleaseInitRequest to decouple config.Config from these structs. Add fields only for the ones that are used. Also reorder fields on these structs alphabetically.
1 parent 973d2df commit a6a3990

File tree

4 files changed

+74
-81
lines changed

4 files changed

+74
-81
lines changed

internal/docker/docker.go

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -66,78 +66,108 @@ type Docker struct {
6666
// BuildRequest contains all the information required for a language
6767
// container to run the build command.
6868
type BuildRequest struct {
69-
// cfg is a pointer to the [config.Config] struct, holding general configuration
70-
// values parsed from flags or environment variables.
71-
Cfg *config.Config
72-
// state is a pointer to the [config.LibrarianState] struct, representing
73-
// the overall state of the generation and release pipeline.
74-
State *config.LibrarianState
75-
// libraryID specifies the ID of the library to build.
69+
// HostMount specifies a mount point from the Docker host into the Docker
70+
// container. The format is "{host-dir}:{local-dir}".
71+
HostMount string
72+
73+
// LibraryID specifies the ID of the library to build.
7674
LibraryID string
75+
7776
// RepoDir is the local root directory of the language repository.
7877
RepoDir string
78+
79+
// State is a pointer to the [config.LibrarianState] struct, representing
80+
// the overall state of the generation and release pipeline.
81+
State *config.LibrarianState
7982
}
8083

8184
// ConfigureRequest contains all the information required for a language
8285
// container to run the configure command.
8386
type ConfigureRequest struct {
84-
// cfg is a pointer to the [config.Config] struct, holding general configuration
85-
// values parsed from flags or environment variables.
86-
Cfg *config.Config
87-
// state is a pointer to the [config.LibrarianState] struct, representing
88-
// the overall state of the generation and release pipeline.
89-
State *config.LibrarianState
90-
// apiRoot specifies the root directory of the API specification repo.
87+
// ApiRoot specifies the root directory of the API specification repo.
9188
ApiRoot string
89+
90+
// HostMount specifies a mount point from the Docker host into the Docker
91+
// container. The format is "{host-dir}:{local-dir}".
92+
HostMount string
93+
9294
// libraryID specifies the ID of the library to configure.
9395
LibraryID string
96+
9497
// RepoDir is the local root directory of the language repository.
9598
RepoDir string
99+
100+
// State is a pointer to the [config.LibrarianState] struct, representing
101+
// the overall state of the generation and release pipeline.
102+
State *config.LibrarianState
96103
}
97104

98105
// GenerateRequest contains all the information required for a language
99106
// container to run the generate command.
100107
type GenerateRequest struct {
101-
// cfg is a pointer to the [config.Config] struct, holding general configuration
102-
// values parsed from flags or environment variables.
103-
Cfg *config.Config
104-
// state is a pointer to the [config.LibrarianState] struct, representing
105-
// the overall state of the generation and release pipeline.
106-
State *config.LibrarianState
107-
// apiRoot specifies the root directory of the API specification repo.
108+
// ApiRoot specifies the root directory of the API specification repo.
108109
ApiRoot string
109-
// libraryID specifies the ID of the library to generate.
110+
111+
// HostMount specifies a mount point from the Docker host into the Docker
112+
// container. The format is "{host-dir}:{local-dir}".
113+
HostMount string
114+
115+
// LibraryID specifies the ID of the library to generate.
110116
LibraryID string
111-
// output specifies the empty output directory into which the command should
117+
118+
// Output specifies the empty output directory into which the command should
112119
// generate code
113120
Output string
121+
114122
// RepoDir is the local root directory of the language repository.
115123
RepoDir string
124+
125+
// State is a pointer to the [config.LibrarianState] struct, representing
126+
// the overall state of the generation and release pipeline.
127+
State *config.LibrarianState
116128
}
117129

118130
// ReleaseInitRequest contains all the information required for a language
119131
// container to run the init command.
120132
type ReleaseInitRequest struct {
121-
// Cfg is a pointer to the [config.Config] struct, holding general configuration
122-
// values parsed from flags or environment variables.
123-
Cfg *config.Config
133+
// Branch is the remote branch of the language repository to use.
134+
Branch string
135+
136+
// Commit determines whether to create a commit for the release but not
137+
// create a pull request. This flag is ignored if Push is set to true.
138+
Commit bool
139+
140+
// HostMount is used to remap Docker mount paths when running in environments
141+
// where Docker containers are siblings (e.g., Kokoro).
142+
// It specifies a mount point from the Docker host into the Docker container.
143+
// The format is "{host-dir}:{local-dir}".
144+
HostMount string
145+
124146
// LibrarianConfig is a pointer to the [config.LibrarianConfig] struct, holding
125147
// global files configuration in a language repository.
126148
LibrarianConfig *config.LibrarianConfig
127-
// State is a pointer to the [config.LibrarianState] struct, representing
128-
// the overall state of the generation and release pipeline.
129-
State *config.LibrarianState
149+
130150
// LibraryID specifies the ID of the library to release.
131151
LibraryID string
152+
132153
// LibraryVersion specifies the version of the library to release.
133154
LibraryVersion string
155+
134156
// Output specifies the empty output directory into which the command should
135157
// generate code.
136158
Output string
159+
137160
// PartialRepoDir is the local root directory of language repository contains
138161
// files that make up libraries and global files.
139162
// This is the directory that container can access.
140163
PartialRepoDir string
164+
165+
// Push determines whether to push changes to GitHub.
166+
Push bool
167+
168+
// State is a pointer to the [config.LibrarianState] struct, representing
169+
// the overall state of the generation and release pipeline.
170+
State *config.LibrarianState
141171
}
142172

143173
// New constructs a Docker instance which will invoke the specified
@@ -184,8 +214,7 @@ func (c *Docker) Generate(ctx context.Context, request *GenerateRequest) error {
184214
fmt.Sprintf("%s:/output", request.Output),
185215
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume
186216
}
187-
188-
return c.runDocker(ctx, request.Cfg.HostMount, CommandGenerate, mounts, commandArgs)
217+
return c.runDocker(ctx, request.HostMount, CommandGenerate, mounts, commandArgs)
189218
}
190219

191220
// Build builds the library with an ID of libraryID, as configured in
@@ -212,7 +241,7 @@ func (c *Docker) Build(ctx context.Context, request *BuildRequest) error {
212241
"--repo=/repo",
213242
}
214243

215-
return c.runDocker(ctx, request.Cfg.HostMount, CommandBuild, mounts, commandArgs)
244+
return c.runDocker(ctx, request.HostMount, CommandBuild, mounts, commandArgs)
216245
}
217246

218247
// Configure configures an API within a repository, either adding it to an
@@ -245,7 +274,7 @@ func (c *Docker) Configure(ctx context.Context, request *ConfigureRequest) (stri
245274
fmt.Sprintf("%s:/source:ro", request.ApiRoot), // readonly volume
246275
}
247276

248-
if err := c.runDocker(ctx, request.Cfg.HostMount, CommandConfigure, mounts, commandArgs); err != nil {
277+
if err := c.runDocker(ctx, request.HostMount, CommandConfigure, mounts, commandArgs); err != nil {
249278
return "", err
250279
}
251280

@@ -277,7 +306,7 @@ func (c *Docker) ReleaseInit(ctx context.Context, request *ReleaseInitRequest) e
277306
fmt.Sprintf("%s:/output", request.Output),
278307
}
279308

280-
if err := c.runDocker(ctx, request.Cfg.HostMount, CommandReleaseInit, mounts, commandArgs); err != nil {
309+
if err := c.runDocker(ctx, request.HostMount, CommandReleaseInit, mounts, commandArgs); err != nil {
281310
return err
282311
}
283312

internal/docker/docker_test.go

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,6 @@ func TestDockerRun(t *testing.T) {
6464
)
6565

6666
state := &config.LibrarianState{}
67-
cfg := &config.Config{}
68-
cfgInDocker := &config.Config{
69-
HostMount: "hostDir:localDir",
70-
}
7167
repoDir := filepath.Join(os.TempDir())
7268
for _, test := range []struct {
7369
name string
@@ -84,7 +80,6 @@ func TestDockerRun(t *testing.T) {
8480
},
8581
runCommand: func(ctx context.Context, d *Docker) error {
8682
generateRequest := &GenerateRequest{
87-
Cfg: cfg,
8883
State: state,
8984
RepoDir: repoDir,
9085
ApiRoot: testAPIRoot,
@@ -115,7 +110,6 @@ func TestDockerRun(t *testing.T) {
115110
},
116111
runCommand: func(ctx context.Context, d *Docker) error {
117112
generateRequest := &GenerateRequest{
118-
Cfg: cfg,
119113
State: state,
120114
RepoDir: "/non-existed-dir",
121115
ApiRoot: testAPIRoot,
@@ -135,7 +129,6 @@ func TestDockerRun(t *testing.T) {
135129
},
136130
runCommand: func(ctx context.Context, d *Docker) error {
137131
generateRequest := &GenerateRequest{
138-
Cfg: cfg,
139132
State: state,
140133
RepoDir: repoDir,
141134
ApiRoot: testAPIRoot,
@@ -156,12 +149,12 @@ func TestDockerRun(t *testing.T) {
156149
},
157150
runCommand: func(ctx context.Context, d *Docker) error {
158151
generateRequest := &GenerateRequest{
159-
Cfg: cfgInDocker,
160152
State: state,
161153
RepoDir: repoDir,
162154
ApiRoot: testAPIRoot,
163155
Output: "hostDir",
164156
LibraryID: testLibraryID,
157+
HostMount: "hostDir:localDir",
165158
}
166159

167160
return d.Generate(ctx, generateRequest)
@@ -187,7 +180,6 @@ func TestDockerRun(t *testing.T) {
187180
},
188181
runCommand: func(ctx context.Context, d *Docker) error {
189182
buildRequest := &BuildRequest{
190-
Cfg: cfg,
191183
State: state,
192184
LibraryID: testLibraryID,
193185
RepoDir: repoDir,
@@ -212,7 +204,6 @@ func TestDockerRun(t *testing.T) {
212204
},
213205
runCommand: func(ctx context.Context, d *Docker) error {
214206
buildRequest := &BuildRequest{
215-
Cfg: cfg,
216207
State: state,
217208
LibraryID: testLibraryID,
218209
RepoDir: "/non-exist-dir",
@@ -230,7 +221,6 @@ func TestDockerRun(t *testing.T) {
230221
},
231222
runCommand: func(ctx context.Context, d *Docker) error {
232223
buildRequest := &BuildRequest{
233-
Cfg: cfg,
234224
State: state,
235225
LibraryID: testLibraryID,
236226
RepoDir: repoDir,
@@ -249,7 +239,6 @@ func TestDockerRun(t *testing.T) {
249239
},
250240
runCommand: func(ctx context.Context, d *Docker) error {
251241
configureRequest := &ConfigureRequest{
252-
Cfg: cfg,
253242
State: state,
254243
LibraryID: testLibraryID,
255244
RepoDir: repoDir,
@@ -306,7 +295,6 @@ func TestDockerRun(t *testing.T) {
306295
},
307296
}
308297
configureRequest := &ConfigureRequest{
309-
Cfg: cfg,
310298
State: curState,
311299
LibraryID: testLibraryID,
312300
RepoDir: repoDir,
@@ -341,7 +329,6 @@ func TestDockerRun(t *testing.T) {
341329
},
342330
runCommand: func(ctx context.Context, d *Docker) error {
343331
configureRequest := &ConfigureRequest{
344-
Cfg: cfg,
345332
State: state,
346333
LibraryID: testLibraryID,
347334
RepoDir: "/non-exist-dir",
@@ -361,7 +348,6 @@ func TestDockerRun(t *testing.T) {
361348
},
362349
runCommand: func(ctx context.Context, d *Docker) error {
363350
configureRequest := &ConfigureRequest{
364-
Cfg: cfg,
365351
State: state,
366352
LibraryID: testLibraryID,
367353
RepoDir: repoDir,
@@ -388,9 +374,6 @@ func TestDockerRun(t *testing.T) {
388374
}
389375

390376
releaseInitRequest := &ReleaseInitRequest{
391-
Cfg: &config.Config{
392-
Repo: repoDir,
393-
},
394377
State: state,
395378
Output: testOutput,
396379
LibrarianConfig: &config.LibrarianConfig{},
@@ -425,9 +408,6 @@ func TestDockerRun(t *testing.T) {
425408
}
426409

427410
releaseInitRequest := &ReleaseInitRequest{
428-
Cfg: &config.Config{
429-
Repo: repoDir,
430-
},
431411
State: state,
432412
PartialRepoDir: partialRepoDir,
433413
Output: testOutput,
@@ -447,9 +427,6 @@ func TestDockerRun(t *testing.T) {
447427
},
448428
runCommand: func(ctx context.Context, d *Docker) error {
449429
releaseInitRequest := &ReleaseInitRequest{
450-
Cfg: &config.Config{
451-
Repo: os.TempDir(),
452-
},
453430
State: state,
454431
PartialRepoDir: "/non-exist-dir",
455432
Output: testOutput,
@@ -471,9 +448,6 @@ func TestDockerRun(t *testing.T) {
471448
t.Fatal(err)
472449
}
473450
releaseInitRequest := &ReleaseInitRequest{
474-
Cfg: &config.Config{
475-
Repo: repoDir,
476-
},
477451
State: state,
478452
PartialRepoDir: partialRepoDir,
479453
Output: testOutput,
@@ -508,9 +482,6 @@ func TestDockerRun(t *testing.T) {
508482
}
509483

510484
releaseInitRequest := &ReleaseInitRequest{
511-
Cfg: &config.Config{
512-
Repo: os.TempDir(),
513-
},
514485
State: state,
515486
PartialRepoDir: partialRepoDir,
516487
Output: testOutput,
@@ -888,9 +859,6 @@ func TestReleaseInitRequestContent(t *testing.T) {
888859
}
889860

890861
req := &ReleaseInitRequest{
891-
Cfg: &config.Config{
892-
Repo: tmpDir,
893-
},
894862
State: stateWithChanges,
895863
PartialRepoDir: partialRepoDir,
896864
Output: filepath.Join(tmpDir, "output"),

internal/librarian/generate.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -321,12 +321,12 @@ func (r *generateRunner) runGenerateCommand(ctx context.Context, libraryID, outp
321321
}
322322

323323
generateRequest := &docker.GenerateRequest{
324-
Cfg: &config.Config{HostMount: r.hostMount},
325-
State: r.state,
326324
ApiRoot: apiRoot,
325+
HostMount: r.hostMount,
327326
LibraryID: libraryID,
328327
Output: outputDir,
329328
RepoDir: r.repo.GetDir(),
329+
State: r.state,
330330
}
331331
slog.Info("Performing generation for library", "id", libraryID, "outputDir", outputDir)
332332
if err := r.containerClient.Generate(ctx, generateRequest); err != nil {
@@ -363,10 +363,10 @@ func (r *generateRunner) runBuildCommand(ctx context.Context, libraryID string)
363363
}
364364

365365
buildRequest := &docker.BuildRequest{
366-
Cfg: &config.Config{HostMount: r.hostMount},
367-
State: r.state,
366+
HostMount: r.hostMount,
368367
LibraryID: libraryID,
369368
RepoDir: r.repo.GetDir(),
369+
State: r.state,
370370
}
371371
slog.Info("Performing build for library", "id", libraryID)
372372
if err := r.containerClient.Build(ctx, buildRequest); err != nil {
@@ -425,11 +425,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context) (string, error
425425
}
426426

427427
configureRequest := &docker.ConfigureRequest{
428-
Cfg: &config.Config{HostMount: r.hostMount},
429-
State: r.state,
430428
ApiRoot: apiRoot,
429+
HostMount: r.hostMount,
431430
LibraryID: r.library,
432431
RepoDir: r.repo.GetDir(),
432+
State: r.state,
433433
}
434434
slog.Info("Performing configuration for library", "id", r.library)
435435
if _, err := r.containerClient.Configure(ctx, configureRequest); err != nil {

0 commit comments

Comments
 (0)