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

[release tool] Slack notifications #9668

Merged
merged 9 commits into from
Jan 13, 2025

Conversation

radTuti
Copy link
Contributor

@radTuti radTuti commented Jan 2, 2025

Description

This modifies how slack notifications are handled with the release tool to surface errors more.

Related issues/PRs

replaces #9350

Todos

  • Tests
  • Documentation
  • Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@radTuti radTuti added docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact labels Jan 2, 2025
@radTuti radTuti requested a review from a team as a code owner January 2, 2025 18:56
@marvin-tigera marvin-tigera added this to the Calico v3.30.0 milestone Jan 2, 2025
@radTuti radTuti force-pushed the slack-notifications branch from 5d877f3 to 4f12d18 Compare January 3, 2025 07:52
@radTuti radTuti force-pushed the slack-notifications branch from 4f12d18 to 764e344 Compare January 3, 2025 07:54
release/cmd/main.go Outdated Show resolved Hide resolved
release/internal/utils/utils.go Outdated Show resolved Hide resolved
release/internal/imagescanner/scanner.go Outdated Show resolved Hide resolved
@radTuti radTuti requested a review from caseydavenport January 3, 2025 23:40
Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

Just minor comments.

metadata.mk Outdated Show resolved Hide resolved
release/cmd/main.go Outdated Show resolved Hide resolved
@radTuti radTuti force-pushed the slack-notifications branch from fef6481 to 48b7f6b Compare January 9, 2025 19:27
@radTuti radTuti requested a review from caseydavenport January 9, 2025 19:27
release/Makefile Outdated
export ORGANIZATION
export GIT_REPO
export GIT_REPO_SLUG
export DEV_REGISTRIES
export OPERATOR_BRANCH

LDFLAGS= -X $(PACKAGE_NAME)/pkg/buildinfo.ProductName='$(PRODUCT_NAME)'
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to pass this in via LDFLAGs? What is the benefit of this over defining it as a constant in Golang?

metadata.mk Outdated Show resolved Hide resolved
@radTuti radTuti force-pushed the slack-notifications branch from f5029d4 to 3f10f1e Compare January 13, 2025 19:46
@radTuti radTuti merged commit 8ac4f70 into projectcalico:master Jan 13, 2025
3 checks passed
@radTuti radTuti deleted the slack-notifications branch January 13, 2025 20:18
radTuti added a commit to radTuti/calico that referenced this pull request Jan 13, 2025
* slack notifications

* allow setting debug via env var

* cleanups

* address review feedback

* Compile product name into binary

* use constant instead of ldflag
radTuti added a commit that referenced this pull request Jan 14, 2025
[release tool] Slack notifications (#9668)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-not-required Docs not required for this change release-note-not-required Change has no user-facing impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants