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.

Sign up for GitHub

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

Jump to bottom

feat(service): add service tech email support #576

Merged
merged 2 commits into from
Jan 11, 2024
Merged

feat(service): add service tech email support #576

merged 2 commits into from
Jan 11, 2024

Conversation

rriski
Copy link
Contributor

@rriski rriski commented Dec 19, 2023

Add support for service tech emails

@rriski rriski requested a review from a team December 19, 2023 10:17
byashimov
byashimov previously requested changes Dec 19, 2023
View reviewed changes
Copy link
Contributor

@byashimov byashimov left a comment

Choose a reason for hiding this comment

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

Please add a test by extending any existing service test. Like Kafka, pg, etc.

api/v1alpha1/common.go Outdated Show resolved Hide resolved
@byashimov
Copy link
Contributor

Btw please make sure that existing list can be purged. It is sufficient to test the behaviour on a local cluster, because the test might be complicated a bit.

@rriski rriski marked this pull request as draft December 19, 2023 12:20
@rriski rriski closed this Jan 8, 2024
@rriski rriski force-pushed the rriski-service-tech-emails branch from 78c2256 to 27aab44 Compare January 8, 2024 12:46
@Serpentiel Serpentiel added the enhancement New feature or request label Jan 9, 2024
@Serpentiel Serpentiel added invalid This doesn't seem right and removed enhancement New feature or request labels Jan 9, 2024
@rriski rriski reopened this Jan 10, 2024
@rriski rriski removed the invalid This doesn't seem right label Jan 10, 2024
@rriski rriski marked this pull request as ready for review January 10, 2024 14:04
@rriski rriski force-pushed the rriski-service-tech-emails branch from 359d1c0 to 16a195a Compare January 10, 2024 14:04
@rriski
Copy link
Contributor Author

rriski commented Jan 10, 2024

Btw please make sure that existing list can be purged. It is sufficient to test the behaviour on a local cluster, because the test might be complicated a bit.

Test case added in PR handles checking that emails can be deleted

@rriski rriski requested a review from byashimov January 10, 2024 14:07
@rriski rriski dismissed byashimov’s stale review January 10, 2024 14:07

Changes addressed

byashimov
byashimov approved these changes Jan 11, 2024
View reviewed changes
Copy link
Contributor

@byashimov byashimov left a comment

Choose a reason for hiding this comment

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

Great job.

@byashimov byashimov merged commit 6257ea0 into main Jan 11, 2024
7 checks passed
@byashimov byashimov deleted the rriski-service-tech-emails branch January 11, 2024 11:16
@Serpentiel Serpentiel added the enhancement New feature or request label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byashimov byashimov byashimov approved these changes

Assignees

@rriski rriski

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rriski @byashimov @Serpentiel