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

Adding first draft of IntegrationSource doc #6156

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Adding first draft of IntegrationSource doc #6156

merged 1 commit into from
Nov 26, 2024

Conversation

matzew
Copy link
Member

@matzew matzew commented Nov 6, 2024

Work in progress, do not yet merge.

/hold
(for now)

@knative-prow knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Nov 6, 2024
@knative-prow knative-prow bot requested review from nainaz and pierDipi November 6, 2024 16:47
@knative-prow knative-prow bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 6, 2024
@netlify Netlify
Copy link

netlify bot commented Nov 6, 2024 •
edited
Loading

✅ Deploy Preview for knative ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 20c7e1d
🔍 Latest deploy log https://app.netlify.com/sites/knative/deploys/67457c4b32e1c40008143465
😎 Deploy Preview https://deploy-preview-6156--knative.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@matzew matzew mentioned this pull request Nov 6, 2024
Open
@matzew matzew force-pushed the integrationsource_v1alpha1 branch from 4add11d to 114fe50 Compare November 25, 2024 16:55
@matzew matzew changed the title WIP: adding first draft of IntegrationSource doc Adding first draft of IntegrationSource doc Nov 25, 2024
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2024
@matzew
Copy link
Member Author

matzew commented Nov 25, 2024

/unhold
IntegrationSource CRD got merged

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 25, 2024
pierDipi
pierDipi reviewed Nov 25, 2024
View reviewed changes
docs/eventing/sources/integration-source/README.md Outdated
Comment on lines 79 to 80

### AWS DynamoDB Streams
Copy link
Member

Choose a reason for hiding this comment

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

I would split pages for each source, that helps with Google search and general discoverability

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. thats possible

Cali0707
Cali0707 reviewed Nov 25, 2024
View reviewed changes
Copy link
Member

@Cali0707 Cali0707 left a comment

Choose a reason for hiding this comment

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

Can we link to this in the table of all sources: https://github.com/knative/docs/tree/main/docs/eventing/sources @matzew ?

@matzew matzew force-pushed the integrationsource_v1alpha1 branch from 114fe50 to 6d07209 Compare November 26, 2024 07:24
@matzew
Copy link
Member Author

matzew commented Nov 26, 2024

Can we link to this in the table of all sources: https://github.com/knative/docs/tree/main/docs/eventing/sources @matzew ?

yes, I will add it. but I generally wanted to rework that page. remove triggermesh, and list ours

@matzew matzew force-pushed the integrationsource_v1alpha1 branch 2 times, most recently from 7dc401d to ad49091 Compare November 26, 2024 07:34
@matzew
Copy link
Member Author

matzew commented Nov 26, 2024

@Cali0707 that page needs a separate PR for cleaning up. there are some inactive sources etc. So I will separate this change.

@matzew matzew force-pushed the integrationsource_v1alpha1 branch from ad49091 to 20c7e1d Compare November 26, 2024 07:44
pierDipi
pierDipi approved these changes Nov 26, 2024
View reviewed changes
Copy link
Member

@pierDipi pierDipi left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

I think we could link to the IntegrationSource API reference in each source page so that we let users know the full spec

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Nov 26, 2024
@dsimansk
Copy link
Contributor

/approve
/lgtm

@knative-prow Knative Prow
Copy link

knative-prow bot commented Nov 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dsimansk, matzew, pierDipi

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 26, 2024
@knative-prow knative-prow bot merged commit ce88f0f into knative:main Nov 26, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Cali0707 Cali0707 Cali0707 left review comments

@pierDipi pierDipi pierDipi approved these changes

@nainaz nainaz Awaiting requested review from nainaz

Assignees

@dsimansk dsimansk

@pierDipi pierDipi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@matzew @dsimansk @pierDipi @Cali0707