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

fix: do not overwrite the entrypoint of nats-server-config-reloader #3004

Merged
merged 8 commits into from
Feb 19, 2024

Conversation

yulin-li
Copy link
Contributor

@yulin-li yulin-li commented Feb 2, 2024

Checklist:

To support distroless based nats-server-config-reloader docker image

The natsio/nats-server-config-reloader image should be >= 0.9.0 now

@ryancurrah
Copy link
Contributor

You need to sign off on your commits. Running git commit -s --amend should do it, then do a git push --force-with-lease.

 -s, --signoff, --no-signoff
           Add a Signed-off-by trailer by the committer at the end of the commit log message. The meaning of a signoff depends on the project to which you’re
           committing. For example, it may certify that the committer has the rights to submit the work under the project’s license or agrees to some contributor
           representation, such as a Developer Certificate of Origin. (See http://developercertificate.org for the one used by the Linux kernel and Git projects.)
           Consult the documentation or leadership of the project to which you’re contributing to understand how the signoffs are used in that project.

           The --no-signoff option can be used to countermand an earlier --signoff option on the command line.

Yulin Li added 2 commits February 3, 2024 09:58
Signed-off-by: Yulin Li <yulili@microsoft.com>
Signed-off-by: Yulin Li <yulili@microsoft.com>
@yulin-li yulin-li marked this pull request as ready for review February 3, 2024 01:59
@yulin-li
Copy link
Contributor Author

yulin-li commented Feb 3, 2024

You need to sign off on your commits. Running git commit -s --amend should do it, then do a git push --force-with-lease.

 -s, --signoff, --no-signoff
           Add a Signed-off-by trailer by the committer at the end of the commit log message. The meaning of a signoff depends on the project to which you’re
           committing. For example, it may certify that the committer has the rights to submit the work under the project’s license or agrees to some contributor
           representation, such as a Developer Certificate of Origin. (See http://developercertificate.org for the one used by the Linux kernel and Git projects.)
           Consult the documentation or leadership of the project to which you’re contributing to understand how the signoffs are used in that project.

           The --no-signoff option can be used to countermand an earlier --signoff option on the command line.

Thanks for reminder, I signed the commit off

Signed-off-by: Yulin Li <yulili@microsoft.com>
@yulin-li yulin-li requested a review from whynowy February 3, 2024 08:20
@yulin-li yulin-li closed this Feb 3, 2024
@yulin-li yulin-li reopened this Feb 3, 2024
Signed-off-by: Yulin Li <yulili@microsoft.com>
@yulin-li yulin-li closed this Feb 3, 2024
@yulin-li yulin-li reopened this Feb 3, 2024
@@ -151,7 +151,7 @@ docs/assets/diagram.png: go-diagrams/diagram.dot
.PHONY: start
start: image
kubectl apply -f test/manifests/argo-events-ns.yaml
kubectl kustomize test/manifests | sed 's@quay.io/argoproj/@$(IMAGE_NAMESPACE)/@' | sed 's/:$(BASE_VERSION)/:$(VERSION)/' | kubectl -n argo-events apply -l app.kubernetes.io/part-of=argo-events --prune=false --force -f -
kubectl kustomize test/manifests | sed 's@quay.io/argoproj/@$(IMAGE_NAMESPACE)/@' | sed 's/argo-events:$(BASE_VERSION)/argo-events:$(VERSION)/' | kubectl -n argo-events apply -l app.kubernetes.io/part-of=argo-events --prune=false --force -f -
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this will make it more accurate, just curious, did you see any issue caused by not having argo-events in the script?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the natsio/nats-server-config-reloader:latest was replaced to natsio/nats-server-config-reloader:<id> in the tests, then the tests failed.

Copy link
Member

@whynowy whynowy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@whynowy whynowy merged commit aa351e4 into argoproj:master Feb 19, 2024
8 checks passed
whynowy pushed a commit that referenced this pull request Jun 14, 2024
…3004)

Signed-off-by: Yulin Li <yulili@microsoft.com>
Co-authored-by: Yulin Li <yulili@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants