Aug 30, 2023 · Aug 29, 2023 · Aug 29, 2023 · Aug 30, 2023 · Aug 30, 2023 · Aug 30, 2023
diff --git a/coderd/coderd.go b/coderd/coderd.go
 }

 mux := drpcmux.New()

 err = proto.DRPCRegisterProvisionerDaemon(mux, &provisionerdserver.Server{
 AccessURL:                   api.AccessURL,
 ID:                          daemon.ID,
 OIDCConfig:                  api.OIDCConfig,
 Database:                    api.Database,
 Pubsub:                      api.Pubsub,
 Provisioners:                daemon.Provisioners,
 GitAuthConfigs:              api.GitAuthConfigs,
 Telemetry:                   api.Telemetry,
 Tracer:                      tracer,
 Tags:                        tags,
 QuotaCommitter:              &api.QuotaCommitter,
 Auditor:                     &api.Auditor,
 TemplateScheduleStore:       api.TemplateScheduleStore,
 UserQuietHoursScheduleStore: api.UserQuietHoursScheduleStore,
 AcquireJobDebounce:          debounce,
 Logger:                      api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
 DeploymentValues:            api.DeploymentValues,
 })
 srv, err := provisionerdserver.NewServer(
 api.AccessURL,
 daemon.ID,
 api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
 daemon.Provisioners,
 tags,
 api.Database,
 api.Pubsub,
 api.Telemetry,
 tracer,
 &api.QuotaCommitter,
 &api.Auditor,
 api.TemplateScheduleStore,
 api.UserQuietHoursScheduleStore,
 api.DeploymentValues,
 debounce,
 provisionerdserver.Options{
 OIDCConfig:     api.OIDCConfig,
 GitAuthConfigs: api.GitAuthConfigs,
 },
 )
 if err != nil {
 return nil, err
 }
 err = proto.DRPCRegisterProvisionerDaemon(mux, srv)
 if err != nil {
 return nil, err
 }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1098,26 +1098,31 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti
		}

		mux := drpcmux.New()

		err = proto.DRPCRegisterProvisionerDaemon(mux, &provisionerdserver.Server{
		AccessURL: api.AccessURL,
		ID: daemon.ID,
		OIDCConfig: api.OIDCConfig,
		Database: api.Database,
		Pubsub: api.Pubsub,
		Provisioners: daemon.Provisioners,
		GitAuthConfigs: api.GitAuthConfigs,
		Telemetry: api.Telemetry,
		Tracer: tracer,
		Tags: tags,
		QuotaCommitter: &api.QuotaCommitter,
		Auditor: &api.Auditor,
		TemplateScheduleStore: api.TemplateScheduleStore,
		UserQuietHoursScheduleStore: api.UserQuietHoursScheduleStore,
		AcquireJobDebounce: debounce,
		Logger: api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
		DeploymentValues: api.DeploymentValues,
		})
		srv, err := provisionerdserver.NewServer(
		api.AccessURL,
		daemon.ID,
		api.Logger.Named(fmt.Sprintf("provisionerd-%s", daemon.Name)),
		daemon.Provisioners,
		tags,
		api.Database,
		api.Pubsub,
		api.Telemetry,
		tracer,
		&api.QuotaCommitter,
		&api.Auditor,
		api.TemplateScheduleStore,
		api.UserQuietHoursScheduleStore,
		api.DeploymentValues,
		debounce,
Copy link Member mafredriAug 29, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. This feels like a lot of args and has it's own risk of mixed up arg order when there are same types (not the case here, now). Typically I'd revert to a struct but I understand why we're making this change. Two structs may be another option. We have a linter that checks that all fields are set (only enabled for certain types atm). Copy link ContributorAuthor spikecurtisAug 29, 2023 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. The "a lot of args" is it's own code smell, and is a symptom of poor code architecture to have so many required args. Making it a struct makes it too easy to leave off required arguments. It doesn't reduce the amount of code, and in fact, increases it. Copy link Member mafredriAug 29, 2023• edited Loading Choose a reason for hiding this comment The reason will be displayed to describe this comment to others.Learn more. Making it a struct makes it too easy to leave off required arguments. It doesn't reduce the amount of code, and in fact, increases it. I think we should value clarity more-so than reducing the number of characters. And it's not like it would increase LOC, just verbosity (but also clarity). But again, feel free to take this as a suggestion, not as an asked change. PS. I was making a case for a linter that detects missing fields. And the struct fields can be further verified in`New` (like some args are now). IMO that levels the playing field.
		provisionerdserver.Options{
		OIDCConfig: api.OIDCConfig,
		GitAuthConfigs: api.GitAuthConfigs,
		},
		)
		if err != nil {
		return nil, err
		}
		err = proto.DRPCRegisterProvisionerDaemon(mux, srv)
		if err != nil {
		return nil, err
		}
Expand Down