Skip to content

fix eventing/samples/helloworld-python#3682

Merged
knative-prow-robot merged 2 commits into
knative:mkdocsfrom
novahe:fix/helloworld-py
Jun 7, 2021
Merged

fix eventing/samples/helloworld-python#3682
knative-prow-robot merged 2 commits into
knative:mkdocsfrom
novahe:fix/helloworld-py

Conversation

@novahe
Copy link
Copy Markdown
Contributor

@novahe novahe commented May 27, 2021 •
edited
Loading

Fixes #3679

Proposed Changes

  • The sample to create a Broker also.
  • Reformat YAML

@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label May 27, 2021
@knative-prow-robot knative-prow-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 27, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented May 27, 2021

Hi @novahe. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow-robot knative-prow-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 27, 2021
@netlify
Copy link
Copy Markdown

netlify Bot commented May 27, 2021 •
edited
Loading

✔️ Deploy Preview for dev-knative ready!

🔨 Explore the source changes: 9495c4b

🔍 Inspect the deploy log: https://app.netlify.com/sites/dev-knative/deploys/60bb92a5124e660007f42e36

😎 Browse the preview: https://deploy-preview-3682--dev-knative.netlify.app

@novahe novahe force-pushed the fix/helloworld-py branch 2 times, most recently from e231bbf to b0b845b Compare May 27, 2021 14:50
@novahe novahe mentioned this pull request May 27, 2021
Merged
@abrennan89 abrennan89 requested a review from vaikas May 27, 2021 18:25
@abrennan89
Copy link
Copy Markdown
Contributor

abrennan89 commented May 27, 2021

/approve

I'll let @vaikas double check and lgtm this since he raised the issue. Thanks @novahe !

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 27, 2021
vaikas
vaikas reviewed May 28, 2021
View reviewed changes
Comment thread docs/eventing/samples/helloworld/helloworld-python/README.md
@knative-prow-robot knative-prow-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 28, 2021
@novahe novahe force-pushed the fix/helloworld-py branch from 528e87f to beb8a38 Compare May 28, 2021 16:11
@novahe novahe requested a review from vaikas May 29, 2021 11:15
@abrennan89 abrennan89 mentioned this pull request Jun 1, 2021
Closed
@abrennan89
Copy link
Copy Markdown
Contributor

abrennan89 commented Jun 1, 2021

/ok-to-test

@knative-prow-robot knative-prow-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jun 1, 2021
csantanapr
csantanapr suggested changes Jun 3, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@csantanapr csantanapr left a comment

Choose a reason for hiding this comment

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

Thank you for helping on this issue and the contribution

Found a few problems in both go and python samples

  1. Link to section send event doesn't work on both
    Replace
1. [Send a CloudEvent to the Broker](###Send-CloudEvent-to-the-Broker)

With

1. [Send a CloudEvent to the Broker](#send-and-verify-cloudevents)
  1. URL for the broker in python sample is wrong
    Replace
curl -v "default-broker.knative-samples.svc.cluster.local"

with

curl -v "broker-ingress.knative-eventing.svc.cluster.local/knative-samples/default"
  1. Event Ce-Type is wrong in Go sample
    Replace
-H "Ce-Type: dev.knative.samples.helloworld"

With:

-H "Ce-Type: dev.knative.samples.hifromknative"

@novahe novahe force-pushed the fix/helloworld-py branch from beb8a38 to 9d4ee19 Compare June 5, 2021 14:23
@knative-prow-robot knative-prow-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jun 5, 2021
@novahe
fix helloworld-python
c1c04df 
add note and annotations

fix link (knative#3679)
@novahe novahe force-pushed the fix/helloworld-py branch from 0771f25 to f7e6178 Compare June 5, 2021 14:55
@novahe novahe force-pushed the fix/helloworld-py branch from f7e6178 to 9495c4b Compare June 5, 2021 15:05
@novahe novahe requested a review from csantanapr June 5, 2021 15:14
abrennan89
abrennan89 approved these changes Jun 7, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@abrennan89 abrennan89 left a comment

Choose a reason for hiding this comment

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

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

knative-prow-robot commented Jun 7, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abrennan89, novahe

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

The pull request process is described here

Details 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

@abrennan89 abrennan89 added this to the v0.24.0 milestone Jun 7, 2021
@knative-prow-robot knative-prow-robot merged commit 76e8f4e into knative:mkdocs Jun 7, 2021
@boramalper boramalper mentioned this pull request Jun 8, 2021
Closed
@novahe novahe deleted the fix/helloworld-py branch June 8, 2021 16:22
RichardJJG pushed a commit to RichardJJG/docs that referenced this pull request Jun 30, 2021
@novahe @RichardJJG
fix eventing/samples/helloworld-python (knative#3682)
420dace 
* fix helloworld-python

add note and annotations

fix link (knative#3679)

* fix helloworld-python (knative#3679)
RichardJJG pushed a commit to RichardJJG/docs that referenced this pull request Jul 1, 2021
@novahe @RichardJJG
fix eventing/samples/helloworld-python (knative#3682)
e18d8af 
* fix helloworld-python

add note and annotations

fix link (knative#3679)

* fix helloworld-python (knative#3679)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vaikas vaikas Awaiting requested review from vaikas

@csantanapr csantanapr Awaiting requested review from csantanapr

1 more reviewer

@abrennan89 abrennan89 abrennan89 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@csantanapr csantanapr

@abrennan89 abrennan89

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

v0.24.0

Development

Successfully merging this pull request may close these issues.

Hello World - Python(docs/eventing/samples/helloworld/helloworld-python/_index.md)

5 participants

@novahe @knative-prow-robot @abrennan89 @csantanapr @vaikas