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

feat: k8s notifier #375

Merged
merged 30 commits into from
Feb 13, 2024
Merged

feat: k8s notifier #375

merged 30 commits into from
Feb 13, 2024

Conversation

tomaszbarwicki
Copy link
Contributor

@tomaszbarwicki tomaszbarwicki commented Nov 23, 2023

PR to add Kubernetes release notification. Feature enhances existing 'release notifier' tool which currently supports PostgreSQL release notification. Added specific k8s notification email template content as well as renamed and updated release notifier GitHub Actions workflow for automation.

Updates eclipse-tractusx/sig-infra#320

@tomaszbarwicki tomaszbarwicki marked this pull request as ready for review November 27, 2023 14:43
FaGru3n
FaGru3n previously approved these changes Nov 29, 2023
Copy link
Contributor

@FaGru3n FaGru3n left a comment

Choose a reason for hiding this comment

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

LGTM but can not verify go section

FaGru3n
FaGru3n previously approved these changes Dec 5, 2023
Copy link
Contributor

@FaGru3n FaGru3n left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@SebastianBezold SebastianBezold left a comment

Choose a reason for hiding this comment

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

I would love to see some automated test for the implementation.
It also seems, like we would send out emails for fix versions.
Looking at the history on the kubernetes release page, it seems like the release is always updated to the latest patch version.

does colly guarantee a correct order for onHTML? The used tag + css class combination is used a lot on that page

@tomaszbarwicki
Copy link
Contributor Author

I would love to see some automated test for the implementation. It also seems, like we would send out emails for fix versions. Looking at the history on the kubernetes release page, it seems like the release is always updated to the latest patch version.

does colly guarantee a correct order for onHTML? The used tag + css class combination is used a lot on that page

Added a basic unit test to verify returned release version is SemVer. Can't think of any other test, feel free to drop any suggestions.
Couldn't find any prove that colly guarantees the order for onHTML hence implemented own order assurance using semver lib.
Included as well in the email content link to step by step guide how to turn off the workflow if there are any issues.

@SebastianBezold
Copy link
Contributor

I would love to see some automated test for the implementation. It also seems, like we would send out emails for fix versions. Looking at the history on the kubernetes release page, it seems like the release is always updated to the latest patch version.
does colly guarantee a correct order for onHTML? The used tag + css class combination is used a lot on that page

Added a basic unit test to verify returned release version is SemVer. Can't think of any other test, feel free to drop any suggestions. Couldn't find any prove that colly guarantees the order for onHTML hence implemented own order assurance using semver lib. Included as well in the email content link to step by step guide how to turn off the workflow if there are any issues.

There is plenty of things that can be tested. For example:

  • notification only sent, if there is actually a new version
  • New version get's persisted, if there is one
  • Sent email contains references to new and old versions

@SebastianBezold
Copy link
Contributor

Hi @tomaszbarwicki,

If you could resolve the merge conflict, then I'll approve the changes

@tomaszbarwicki
Copy link
Contributor Author

Hi @tomaszbarwicki,

If you could resolve the merge conflict, then I'll approve the changes

@SebastianBezold done

@tomaszbarwicki tomaszbarwicki merged commit b0eefb7 into main Feb 13, 2024
5 checks passed
@tomaszbarwicki tomaszbarwicki deleted the feat/k8s_notifier branch February 13, 2024 09:20
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.

3 participants