Skip to content

Revert changes to community#3757

Merged
knative-prow-robot merged 1 commit into
knative:mkdocsfrom
julz:backtothecommunity
Jun 9, 2021
Merged

Revert changes to community#3757
knative-prow-robot merged 1 commit into
knative:mkdocsfrom
julz:backtothecommunity

Conversation

@julz
Copy link
Copy Markdown
Contributor

@julz julz commented Jun 9, 2021

This commit reverts changes made to community which assumed it was being built by mkdocs (since it's still built with hugo) and caused formatting to break in the community tab.

/assign @omerbensaadon I did a straight git checkout HEAD~10 community/ so please check I didn't revert something we actually wanted here, but it looks right to me

@google-cla google-cla Bot added the cla: yes Indicates the PR's author has signed the CLA. label Jun 9, 2021
@knative-prow-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: julz

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 added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 9, 2021
@netlify
Copy link
Copy Markdown

netlify Bot commented Jun 9, 2021

✔️ Deploy Preview for dev-knative ready!

🔨 Explore the source changes: 97d6a69

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

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

@omerbensaadon
Copy link
Copy Markdown

Can you do a find and replace for shell -> bash?

@julz
Copy link
Copy Markdown
Contributor Author

julz commented Jun 9, 2021

Can you do a find and replace for shell -> bash?

Do we care about applying these rules to community? Couldn't we/shouldn't we leave that folder till after GA when we fix up community to use mkdocs?

Either way, if we do want to do that again I'd suggest we re-apply it in a different PR so each PR does one thing as much as possible: having PRs making lots of unrelated changes made this a bit harder to track down via git bisect than it needed to be

@omerbensaadon
Copy link
Copy Markdown

Sorry, I wasn't clear, it was already fixed but this PR changes it back to shell, so it's a regressive change

@omerbensaadon
Copy link
Copy Markdown

/hold
/lgtm

@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 Jun 9, 2021
@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 9, 2021
@omerbensaadon
Copy link
Copy Markdown

@julz I will leave it up to you just in case you did understand my first message as I meant it!

@julz
Copy link
Copy Markdown
Contributor Author

julz commented Jun 9, 2021

I personally think we should probably just leave community/ alone for now, but if it was intentional to fix shell->bash there as well as elsewhere I'm happy to do that conversion again in a follow-on PR once this lands. (It took longer than it should have to track this bug down, and it was harder to revert cleanly and safely, because the commit that introduced the problem made lots of changes at once, so I'd like to avoid repeating that in the fix iyswim).

/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 Jun 9, 2021
@julz
Copy link
Copy Markdown
Contributor Author

julz commented Jun 9, 2021

as soon as this merges Ill do a follow-on to s/shell/bash/ in community, and we can double check that all looks right with hugo (I assume it does, but since the broader change this came in with actually broke community and we didn't realise, we should check!)

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

3 participants