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: add gitea notifier #451

Merged
merged 1 commit into from
Dec 14, 2022
Merged

feat: add gitea notifier #451

merged 1 commit into from
Dec 14, 2022

Conversation

ttys3
Copy link
Contributor

@ttys3 ttys3 commented Dec 4, 2022 •
edited

add support to send event notification to Gitea repo commit status

image

Gitea is a forge software package for hosting software development version control using Git as well as other collaborative features like bug tracking, code review, kanban boards, tickets, and wikis. It supports self-hosting but also provides a free public first-party instance

https://github.com/go-gitea/gitea

@ttys3 ttys3 force-pushed the add-gitea-notifier branch 5 times, most recently from 0e95f42 to 2f1df5c Compare December 4, 2022 03:29
makkes
makkes requested changes Dec 4, 2022
View reviewed changes
internal/notifier/gitea.go Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea.go Outdated Show resolved Hide resolved
internal/notifier/gitea_test.go Outdated Show resolved Hide resolved
internal/notifier/gitea_test.go Show resolved Hide resolved
@ttys3 ttys3 requested a review from makkes December 4, 2022 10:19
@ttys3
Copy link
Contributor Author

ttys3 commented Dec 4, 2022 •
edited

the tests failed. it complains about config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml which I added gitea

@makkes could you tell me how to fix the CI issue ?

@kingdonb
Copy link
Member

kingdonb commented Dec 7, 2022

I have an idea @ttys3 – it looks like you need to run make generate and commit the result. The error is a "dirty working tree" which just means that something in the e2e job runs make generate and you're supposed to already have run this.

I tried that locally but it only came up with half of the changes that failing test mentioned, the other one in go.sum

I guess that make test runs all of that

I found both the change in go.sum and in notification[..]providers.yaml like you mentioned. If you run the tests locally, you should get those changes in your working tree then you can commit them and e2e should hopefully pass after that 🎉

@ttys3
Copy link
Contributor Author

ttys3 commented Dec 8, 2022

@kingdonb make test will remove gitea from config/crd/bases/notification.toolkit.fluxcd.io_providers.yaml

make generate is right.

I need add gitea to api/v1beta1/provider_types.go, the yaml is generated from the annotation

@ttys3
Copy link
Contributor Author

ttys3 commented Dec 8, 2022

OK. fixed

PTAL

@makkes
Copy link
Member

makkes commented Dec 8, 2022

@ttys3 please squash the commits into one.

@makkes makkes self-requested a review December 8, 2022 14:01
@ttys3
Copy link
Contributor Author

ttys3 commented Dec 8, 2022

squash done.

makkes
makkes previously requested changes Dec 9, 2022
View reviewed changes
Copy link
Member

@makkes makkes left a comment

Choose a reason for hiding this comment

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

@stefanprodan actually reminded me that we need to update the docs accordingly here. @ttys3 please add documentation about this new type to the provider documentation page.

@ttys3
Copy link
Contributor Author

ttys3 commented Dec 9, 2022

@makkes added. PTAL

@ttys3 ttys3 requested a review from makkes December 9, 2022 15:25
@stefanprodan
Copy link
Member

@ttys3 this need to be ported to the v1beta2 API, we've deprecated v1beta1 in the main branch and that version is now freezed.

@ttys3 ttys3 force-pushed the add-gitea-notifier branch 3 times, most recently from b8ee136 to c6be6f8 Compare December 13, 2022 16:36
@ttys3 ttys3 force-pushed the add-gitea-notifier branch 2 times, most recently from 77d20e4 to 58d8f3b Compare December 13, 2022 17:00
@ttys3 ttys3 requested review from stefanprodan and removed request for makkes December 13, 2022 17:02
@ttys3
feat: add gitea notifier
d302029 
Signed-off-by: ttyS3 <ttys3.rust@gmail.com>
@ttys3
Copy link
Contributor Author

ttys3 commented Dec 13, 2022

CI failed. force pushed.
PTAL

stefanprodan
stefanprodan approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @ttys3 🥇

@stefanprodan stefanprodan dismissed makkes’s stale review December 14, 2022 08:17

change request was addressed

makkes
makkes approved these changes Dec 14, 2022
View reviewed changes
Copy link
Member

@makkes makkes 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 🎖️

@makkes makkes merged commit 9d3339d into fluxcd:main Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@makkes makkes makkes approved these changes

@stefanprodan stefanprodan stefanprodan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ttys3 @kingdonb @makkes @stefanprodan