Skip to content

fixing broken links#4014

Merged
knative-prow-robot merged 4 commits into
knative:mkdocsfrom
psschwei:link-fix
Jul 26, 2021
Merged

fixing broken links#4014
knative-prow-robot merged 4 commits into
knative:mkdocsfrom
psschwei:link-fix

Conversation

@psschwei
Copy link
Copy Markdown
Member

Fixes #4013

Proposed Changes

  • Fixes broken links on docs/serving/knative-kubernetes-services.md page
    (note that the first link here was circular... I assume it was meant to point to the readme)
  • Fixes broken links in DEVELOPMENT.md

@netlify
Copy link
Copy Markdown

netlify Bot commented Jul 14, 2021

✔️ Deploy Preview for knative ready!

🔨 Explore the source changes: e5c3601

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

😎 Browse the preview: https://deploy-preview-4014--knative.netlify.app/development/reference/api/serving-api

@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Jul 14, 2021
@knative-prow-robot knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jul 14, 2021
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 lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 16, 2021
@abrennan89
Copy link
Copy Markdown
Contributor

/hold

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 16, 2021
@abrennan89
Copy link
Copy Markdown
Contributor

@psschwei this looks like it needs a rebase, also, does this need to be cherrypicked to 0.24 as well?

@psschwei
Copy link
Copy Markdown
Member Author

Yes, this should be cherrypicked to 0.24, as the links are broken there as well

Will rebase shortly.

@knative-prow-robot knative-prow-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jul 16, 2021
@psschwei
Copy link
Copy Markdown
Member Author

/hold

@psschwei
Copy link
Copy Markdown
Member Author

@abrennan89 I think the last two links (to website / maintainer help) on DEVELOPMENT.md have been sunset, maybe as part of #4020 or #4024 . I wonder if it would be better for it to mirror the changes @RichardJJG made to blog/README.md?

@abrennan89 abrennan89 requested a review from RichardJJG July 16, 2021 20:03
@abrennan89
Copy link
Copy Markdown
Contributor

abrennan89 commented Jul 16, 2021

Assigning @RichardJJG to take a look - might be better to rebase once any of his PRs are merged?
@psschwei if this is undoing any of Richard's changes I'd just re-add whatever he did.

@psschwei
Copy link
Copy Markdown
Member Author

psschwei commented Jul 16, 2021

@psschwei if this is undoing any of Richard's changes I'd just re-add whatever he did.

Not undoing any changes, at least I don't think it is. I think basically what's happening is:

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2021
@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 22, 2021
@abrennan89
Copy link
Copy Markdown
Contributor

cc @omerbensaadon @RichardJJG to answer the comments / questions on this one please

@abrennan89 abrennan89 removed their assignment Jul 22, 2021
@omerbensaadon
Copy link
Copy Markdown

@psschwei it might be easier to revert changes to the DEVELOPMENT.md file and just ship the fix the linked issue is targeting

Copy link
Copy Markdown

@omerbensaadon omerbensaadon left a comment

Choose a reason for hiding this comment

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

I think if you remove this it should be good to merge? If you don't have any conflicts with Richard's changes, I'd /unhold.

If you do, I'd take @RichardJJG's changes where there are conflicts.

Comment thread DEVELOPMENT.md Outdated
Comment on lines +21 to +22
* [Website help](https://knative.dev/docs/help/contributor/publishing)
* [Maintainer help](https://knative.dev/docs/help/maintainer/)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

These pages don't exist anymore

Suggested change
* [Website help](https://knative.dev/docs/help/contributor/publishing)
* [Maintainer help](https://knative.dev/docs/help/maintainer/)

@psschwei
Copy link
Copy Markdown
Member Author

It looks like #4054 also fixes the original broken links (and in a better way than my PR), so happy to close this one and let the other one merge instead.

(And apologies to @snneji for forgetting to assign the issue)

@omerbensaadon
Copy link
Copy Markdown

I think your PR still does fix some broken links in Development.MD as well as the blog README

@omerbensaadon
Copy link
Copy Markdown

If you can

  1. Remove the last two lines from my review
  2. Remove changes covered by Fix broken links #4054

We can get this merged

@knative-prow-robot knative-prow-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 23, 2021
@RichardJJG
Copy link
Copy Markdown
Contributor

/lgtm

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

/approve

@psschwei
Copy link
Copy Markdown
Member Author

/hold

Looks like both @abrennan89 and I put holds on this... I'm good to remove mine.

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
/unhold

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abrennan89, psschwei, RichardJJG

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

@knative-prow-robot knative-prow-robot merged commit 4771c4e into knative:mkdocs Jul 26, 2021
RichardJJG added a commit to RichardJJG/docs that referenced this pull request Jul 27, 2021
@psschwei psschwei deleted the link-fix branch November 4, 2021 15:07
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 Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Broken link on Kubernetes services page

5 participants