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

Fix README, examples, and re-enable yaml tests 😅 #477

Merged
merged 2 commits into from
Feb 6, 2019

Conversation

bobcatfish
Copy link
Collaborator

@bobcatfish bobcatfish commented Feb 6, 2019

I tried to run the examples this morning and found that the docs were
wrong and also that the path in the example pipelines was now incorrect.

In 411bc55 we disabled the pipelinerun tests due to the PVC issue, but now that #443 is done we should be able to re-enable them hopefully! 🎉

@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label Feb 6, 2019
@knative-prow-robot knative-prow-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 6, 2019
@bobcatfish bobcatfish changed the title Fix README and examples 😅 Fix README, examples, and re-enable yaml tests 😅 Feb 6, 2019
Copy link
Contributor

@shashwathi shashwathi 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 Feb 6, 2019
@knative-prow-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bobcatfish, shashwathi

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:
  • OWNERS [bobcatfish,shashwathi]

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

@shashwathi
Copy link
Contributor

This PR needs fix from #479

I tried to run the examples this morning and found that the docs were
wrong and also that the path in the example pipelines was now incorrect.
I can see how the docs could get out of sync but something odd is
happening with the pipeline paths b/c our tests should be catching that,
so I'll open a bug. In the meantime, these changes make it so we can at
least successfully create the pipelinerun (also tried the taskrun, which
works - I haven't tried the other examples).
Verifying the pipelinerun example worked was disabled due to the combo
of tektoncd#375 and tektoncd#443, but now that we've removed the extra log PVC in tektoncd#443
we shouldn't run into this issue anymore :D
@knative-prow-robot knative-prow-robot removed the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2019
@bobcatfish
Copy link
Collaborator Author

Rebased hopefully that does it 🤞

@shashwathi
Copy link
Contributor

This PR needs #475 fix as well(I thought).

@shashwathi
Copy link
Contributor

/lgtm

@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 6, 2019
@bobcatfish
Copy link
Collaborator Author

hm the timeout tests are failing, interesting, maybe related to #456? (did we already make the resync change 🤔 )

@shashwathi
Copy link
Contributor

/test pull-knative-build-pipeline-integration-tests

@shashwathi
Copy link
Contributor

We have not added the resync change. I have triggered it again to see if this is a flaky test or valid error.

@knative-prow-robot knative-prow-robot merged commit 9e14e1e into tektoncd:master Feb 6, 2019
@bobcatfish
Copy link
Collaborator Author

I have triggered it again to see if this is a flaky test or valid error.

😥

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. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants