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 business id to cronjob chart #156

Conversation

abdullah-sl
Copy link
Contributor

No description provided.

@abdullah-sl abdullah-sl changed the title Feature/plat 992 add business id to cronjob chart Add business id to cronjob chart Jul 19, 2024
@@ -1,6 +1,6 @@
apiVersion: v1
description: Helm chart with simple cronjob template
name: cronjob
version: 0.9.0
version: 0.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

remember update version before merge

@@ -10,6 +10,8 @@ kind: CronJob
metadata:
name: {{ .Values.name }}
namespace: {{ .Release.Namespace }}
labels:
businessid: {{ required "businessid must be provided" .Values.businessid | quote }}
Copy link
Contributor

@ThomasKwan-shopline ThomasKwan-shopline Jul 23, 2024

Choose a reason for hiding this comment

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

why add required here, should add required in all businessid field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the entry point, so I believe checking here would be enough.

neal-sl
neal-sl previously approved these changes Aug 7, 2024
@thomaslishopline thomaslishopline merged commit 1c9c17f into shoplineapp:master Aug 8, 2024
5 checks passed
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.

4 participants