Stop running make after every command call

[Adirio]

Background

Post-scaffold make

Currently, every init, create api or create webhook call make as the last step. There are some cases where this behavior can be disabled with a flag (v2/create-api, v3/create-api & v3/create-webhook) and others can't even be disabled (v2/init, v3/init and v3/create-webhook).

make functionality

A call to make does four things:

1. Create DeepCopy, DeepCopyTo, and DeepCopyObject methods for your types.
2. go fmt
3. go vet
4. Build the manager

Subcommands functionality evaluation

I will evaluate the need for those four steps for each of the commands.

init

1. No type created yet, no-op.
2. Only scaffolded code, no-op*.
3. Only scaffolded code, no-op*.
4. No types nor controllers, useless manager built.

1, 2 and 3 are no-ops, and the manager built by 4 is completely useless. There is no reason to make after init.

create api

1. If you created a type you need to update it, so you will need to regenerate this afterwards. If you didn't create a type, no use on regenerating the methods.
2. Only scaffolded code, no-op*.
3. Only scaffolded code, no-op*.
4. You just created a type/controller, you will need to code parts (type spec or reconcile loop), so you will need to rebuild afterwards.

1 and 4 will have to be done after editing and 2 and 3 are no-ops. The only reason I see to do this is to have the DeepCopy, DeepCopyTo and DeepCopyObject methods recognized by IDEs while you are writting the code, which would only require make generate and only in the case a new API was scaffolded.

create webhook

1. You didn't create a type, no use on regenerating the methods.
2. Only scaffolded code, no-op*.
3. Only scaffolded code, no-op*.
4. You just created a webhook, you will need to code parts (interface methods), so you will need to rebuild afterwards.

1 has no use, 2 and 3 are no-ops, and 4 will have to be done after editing. There is no reason to make after init.

• If steps 2 or 3 are not no-ops, it means that we are scaffolding things with style errors which should be fixed in the plugins and having this post-scaffold go fmt and go vet will just hide those style errors.

Conclusions

So the basic conclusion is that running make after any of these command calls is useless. You are either doing work that was already done or that you will need to do after editing the scaffolded files. Only make generate after create api --resource has a very small positive effect of having IDEs recognize this methods. However, the user will still need to make generate after adding any field.

Proposal

• Remove make call from all post-scaffold method
• Add a make generate call to the create api post scaffold method in case a new type file is generated.
• Remove the --make flag in v3/create-webhooks

I think we should do this to go/v2 and go/v3. It won't have a real impact on the user so I think they should be considered backwards compatible and thus applied to both plugins Removing the --make flag only needs to be applied to go/v3 and we haven't released any stable version of the go/v3 plugin so I think we should be able to do it.

/kind feature

[Adirio]

While the time impact of a single call may be considered small, this will have a bigger impact in generating the testdata (both for the developer and the CI) and the potential re-scaffolding from a config file feature that we have talked about where multiple calls to the commands are done once after another and for each one we are generating, formating, vet-ing and building (and even at the end we do another make all test which will do the four steps twice more, one for all and one for test which additionally will run the tests).

[camilamacedo86]

Hi @Adirio,

• The make in the post-action does not take more than a few seconds to run in the cases raised as useless.
• For v3 we have the make option as false in all scenarios (which mainly is a valid feature for the CI. People does not run these commands many times. you create a project, does a scaffold and it is done)
• I am afraid that does not agree with your IDE argument because of a lot of the user does not use an IDE at all. Note that a lot of them since a lot of people that work with Kubernetes has a profile of more system admins than developers and many devs work with vim and etc. So, we cannot take any decision-based in the fact of the IDE solves the problem.

Then, what is the problem that you are trying to solve here? Are you looking for win a few seconds of performance for operations that are executed once or twice in the life?

In this way, IMO I do no think that we should move forward with because I cannot see the problem that will be solved with this change. I think it only might introduce an issue in a scenario where would be valid/required runs the make targets and it was not as complexities to define the list of the specific targets that should be executed for each subcommand.

[DirectXMan12]

The make in the post-action does not take more than a few seconds to run in the cases raised as useless.

I've hit cases where this isn't true (especially in resource-constrained situations like CI), and have had situations where I wanted to switch some stuff before make was run, which would've otherwised cause make to fail or do something undesirable (try and pull in new deps, etc)

I am afraid that does not agree with your IDE argument because of a lot of the user does not use an IDE at all.

Perhaps I'm misreading, but my impression was that @Adirio was actually arguing that currently the only users who benefit from make are the IDE users (and only a bit), so removing make only would impact them, not that IDEs somehow solve this issue.

FWIW, I'm on board at least for v3. I think it's prob fine for v2 as well -- I've never been super into running the make command after every invocation.

[Adirio]

Answer to @camilamacedo86

• The make in the post-action does not take more than a few seconds to run in the cases raised as useless.

Even if it was 100ms, why should we waste that time to do nothing? There is no advantage on running them.

• For v3 we have the make option as false in all scenarios (which mainly is a valid feature for the CI. People does not run these commands many times. you create a project, does a scaffold and it is done)

Make flag defaults to true in both plugins. We are manually turning it to false for create api in the CI. But thats still not the point. The point is that running them does nothing for us.

• I am afraid that does not agree with your IDE argument because of a lot of the user does not use an IDE at all. Note that a lot of them since a lot of people that work with Kubernetes has a profile of more system admins than developers and many devs work with vim and etc. So, we cannot take any decision-based in the fact of the IDE solves the problem.

You didn't understood this part right. The IDE argument is the only advantage for using the make. And that only requires make generate not make. It is not an argument to remove them, is the only argument to leave them as they are.

Then, what is the problem that you are trying to solve here? Are you looking for win a few seconds of performance for operations that are executed once or twice in the life?

Thats not the point, the point is that those operation are completely useless and that's what Im trying to prove.

In this way, IMO I do no think that we should move forward with because I cannot see the problem that will be solved with this change. I think it only might introduce an issue in a scenario where would be valid/required runs the make targets and it was not as complexities to define the list of the specific targets that should be executed for each subcommand.

I already listed all the scenarios that can happen when you run any of those commands. You will always have to do additional changes like writting code yourself and call make manually.

Answer to @DirectXMan12

Exactly, the IDE is the only minimal advantage of keeping the tests, and it only applies to make generate on create api --resource. In the PR I still kept the make generate call in this case.

[camilamacedo86]

Thank you for the clarifications. I misunderstood your point of the IDE for the first moment.
So, I think it is fine.