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

📖 Raising event doc page created #3450

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

Sajiyah-Salat
Copy link
Contributor

Description: Add the documentation to help out users know how to implement and raise events.
NOTE: we had this document in v1 docs but it is outdated and we are shaping it accordingly.
This is #3375 original pr but during squashing commits I messed up the original file and this is backup.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jun 13, 2023
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jun 13, 2023
@Sajiyah-Salat Sajiyah-Salat changed the title Raising event doc page created 📖 Raising event doc page created Jun 13, 2023
@Sajiyah-Salat
Copy link
Contributor Author

Sajiyah-Salat commented Jun 13, 2023

Looks like perfect to merge, But why its failing WDYT?

```

### Allowing usage of EventRecorder on the Controller
To raise an event, you must have access to `record.EventRecorder` in the Controller. Therefore, firstly let's update the controller implementation:
Copy link
Member

Choose a reason for hiding this comment

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

Nit: space between title and text.

Recorder record.EventRecorder
}
### Passing the EventRecorder to the Controller
Events are published from a Controller using an [EventRecorder]`type CorrelatorOptions struct`,
Copy link
Member

Choose a reason for hiding this comment

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

Same

Copy link
Contributor Author

@Sajiyah-Salat Sajiyah-Salat Jun 13, 2023

Choose a reason for hiding this comment

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

Got it.

## How to be able to raise Events?

Following are the steps with examples to help you raise events in your controller's reconciliations.
Events are published from a Controller using an [EventRecorder]`type CorrelatorOptions struct`,
Copy link
Member

Choose a reason for hiding this comment

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

It is missing the link:

Screenshot 2023-06-13 at 06 34 19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add this or any other link

Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

@Sajiyah-Salat,

See that it has a few nits.
Since they are not a blocker and you are working on this one for a long period I will

/approved
/lgtm

But if you have a chance please address them in a follow up.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 13, 2023
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, Sajiyah-Salat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 13, 2023
@k8s-ci-robot k8s-ci-robot merged commit 0f390a9 into kubernetes-sigs:master Jun 13, 2023
20 of 21 checks passed
@Sajiyah-Salat
Copy link
Contributor Author

Sajiyah-Salat commented Jun 13, 2023

sorry for issues I will make changes as soon as possible
Btw thank you for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants