Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add real versions to controller main.go #321

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

jaypipes
Copy link
Collaborator

@jaypipes jaypipes commented Apr 6, 2022

I'd left a TODO in the main.go template for controllers to fill in their
GitVersion, GitCommit and BuildDate information properly. Just now
getting around to this...

Issue: aws-controllers-k8s/community#1171

Signed-off-by: Jay Pipes jaypipes@gmail.com

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

I'd left a TODO in the main.go template for controllers to fill in their
GitVersion, GitCommit and BuildDate information properly. Just now
getting around to this...

Issue: aws-controllers-k8s/community#1171

Signed-off-by: Jay Pipes <jaypipes@gmail.com>
@vijtrip2
Copy link
Contributor

vijtrip2 commented Apr 6, 2022

/approve

@RedbackThomson feel free to merge if you have no concerns.

@RedbackThomson
Copy link
Contributor

/approve

@RedbackThomson feel free to merge if you have no concerns.

I'd prefer not to have to modify every repositories Makefiles. I would rather we have them code generated tbh

@vijtrip2
Copy link
Contributor

vijtrip2 commented Apr 6, 2022

I'd prefer not to have to modify every repositories Makefiles. I would rather we have them code generated tbh

I think we won't need to. This PR will correctly use LDFlags from controller/pkg/version in main.go and the Dockerfile correctly sets those LDFLAGS when controller binary is built.

The LDFlags in controller Makefile are not used anywhere because binary is built in DockerFile which does not use Makefile.

@RedbackThomson
Copy link
Contributor

Oh woah I did not notice that! That's confusing, but yay. I'm happy, then!
/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 6, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Apr 6, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jaypipes, RedbackThomson, vijtrip2

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 [RedbackThomson,jaypipes,vijtrip2]

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

@ack-bot ack-bot merged commit 9191f9d into aws-controllers-k8s:main Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants