search

strimzi / strimzi-kafka-operator

Apache Kafka® running on Kubernetes
https://strimzi.io/
Apache License 2.0
4.75k stars 1.27k forks source link

Review whether generated files need to be comitted #1749

Closed tombentley closed 2 years ago

tombentley commented 5 years ago

As shown by #1747, it is confusing to new contributors that we commit (and check for) derived resources being committed to git. I don't recall the original motivation for this, but if files are generated by the build then there should be no need to commit them.

Unless there's a solid reason otherwise, I think we should removed these derived files from git and remove the check.

scholzj commented 5 years ago

The generating of many of these files is too complicated and would often mean that users need to build the project just to use it. Given it would require Java, Maven and Make and not work properly on Windows, having these files committed makes more sense to me from the overall simplicity prespective.

On Fri, Jun 28, 2019, 03:41 Tom Bentley notifications@github.com wrote:

As shown by #1747 https://github.com/strimzi/strimzi-kafka-operator/pull/1747, it is confusing to new contributors that we commit (and check for) derived resources being committed to git. I don't recall the original motivation for this, but if files are generated by the build then there should be no need to commit them.

Unless there's a solid reason otherwise, I think we should removed these derived files from git and remove the check.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/strimzi/strimzi-kafka-operator/issues/1749?email_source=notifications&email_token=ABLFOR4NFEJIOJTLL6KO3L3P4W6B5A5CNFSM4H4C6PEKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G4ICGYQ, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLFOR623XSTQGW5GOSBCRLP4W6B5ANCNFSM4H4C6PEA .

tombentley commented 5 years ago

would often mean that users need to build the project just to use it.

That's pretty normal for a repo on github.

Who do we think is cloning the repo without the intention of building it?

scholzj commented 5 years ago

Who do we think is cloning the repo without the intention of building it?

I assume all the users on Slack asking questions with latest are using it. You are saying that it is too complicated for developers to contribute because of this. But: a) This will force all users to actually do the same just to use master b) Devs will still need to do it to test their changes properly.

tombentley commented 5 years ago

OK then.

I wonder if git adding the generated files as part of the make build would help, or whether new devs would it that confusing that stuff was in their index which they'd not added themselves.

scholzj commented 5 years ago

I wonder if git adding the generated files as part of the make build would help, or whether new devs would it that confusing that stuff was in their index which they'd not added themselves.

You mean that the make or mvn commands would automatically do git add?

tombentley commented 5 years ago

Yes. make would be easier, but I guess it wouldn't catch people doing a pure maven build. I guess we could use the mvn exec plugin to do it via maven though. If it's not too weird.

scholzj commented 5 years ago

I think we will need to be careful so that it doesn't add some unwanted files etc. But that hould be ok I guess. Also, it remains to be seen whether people don't run the calls today (e.g. make helm_install for the PR which triggered this issue) or don't commit the files. My vote is for the first one in which case doing git add there would not help.

scholzj commented 2 years ago

Triaged on 10.5.2022: The generated files are fairly small, we have checks in the CI to make sure they are up to date and the files are useful in some situations. Should be closed.