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

Update website/update-imported-docs/update-imported-docs script for Python 3, PyYAML 5.1.2 #16681

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,20 @@ reference documentation for tools and components in the

* You need a machine that is running Linux or macOS.

* You need to have this software installed:
* Install the following:

* [Python 2.7.16](https://www.python.org/downloads/)
* [Python 3.7.x](https://www.python.org/downloads/)
* [Git](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git)
* [Golang](https://golang.org/doc/install) version 1.12 for Kubernetes 1.14+; Go 1.13 [is not supported](https://github.com/kubernetes/community/blob/master/contributors/devel/development.md#go)
* [PyYAML](https://pyyaml.org/) v3.13
* [Pip](https://pypi.org/project/pip/), which is used to install PyYAML (not installed by default on Ubuntu)
Copy link
Contributor

@kbhawkey kbhawkey Oct 22, 2019

Choose a reason for hiding this comment

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

@aimeeu, this is a nit: I would rather not see the comment about "... not installed ... on Ubuntu"). It seems clear just to say why Pip is a requirement.

Another nit,line 51: Is the $ needed:
$github.com/website. The remaining steps refer to your base directory as <web-base>.

Last nit 😃 : I may have brought this up before, but the Fixing Links section is confusing.
IIRC, this flag should be used to fix the release notes, not the generated markdown files.
Not sure if this script is even used to post process the release notes.

Copy link
Contributor Author

@aimeeu aimeeu Oct 22, 2019

Choose a reason for hiding this comment

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

@kbhawkey I agree. I removed that yesterday early afternoon. Hmmmmm.... it was after I rebased to remove a merge conflict. I have no idea why the doc change didn't get to pushed to the PR when I pushed.
My local copy:

    * [Python 3.7.x](https://www.python.org/downloads/)
    * [Git](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git)
    * [Golang](https://golang.org/doc/install) version 1.12 for Kubernetes 1.14+; Go 1.13 [is not supported](https://github.com/kubernetes/community/blob/master/contributors/devel/development.md#go)
    * [Pip](https://pypi.org/project/pip/), which is used to install PyYAML
    * [PyYAML](https://pyyaml.org/) v5.1.2
    * [make](https://www.gnu.org/software/make/)
    * [gcc compiler/linker](https://gcc.gnu.org/)

and git status says "nothing to commit."

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow -- there are a lot of commits on the last commit. Did you rebase, something like, git fetch upstream,
git rebase -i upstream/master?

Copy link
Contributor Author

@aimeeu aimeeu Oct 22, 2019

Choose a reason for hiding this comment

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

Yes I did. Followed the instructions in the Intermediate contribute doc, and looked at the Community docs as well. Plus now there are two commits, which means I will have to squash, which means more rebasing. SIGH.

Copy link
Contributor

Choose a reason for hiding this comment

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

😬

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure what happened, but the second commit seems to contain the latest change to that file?
At this pt, it may be worthwhile to get these changes in and correct the few remaining things in a second PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm abandoning this PR after I incorporate changes into a new one. I tried to revert the merge commit, which ended in total disaster.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not merge this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not able to squash the commits. However, you are an approver so if you want to merge it, that's fine. Otherwise, I have a new PR ready to go. Let me know what you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since I have no idea what happened, I suppose you should create another PR.
Based upon what I see in the preview and the source, the changes seem fine.

* [PyYAML](https://pyyaml.org/) v5.1.2
* [make](https://www.gnu.org/software/make/)
* [gcc compiler/linker](https://gcc.gnu.org/)

* The Go binary must be in your path; **do not** set your `$GOPATH`. The `update-imported-docs` tool sets your GOPATH.
* The `Go` binary must be in your path. The `update-imported-docs` tool sets your GOPATH.

* You need to know how to create a pull request to a GitHub repository.

This involves creating your own fork of the repository. For more
information, see [Work from a local clone](/docs/contribute/intermediate/#work_from_a_local_clone).

Expand Down
182 changes: 0 additions & 182 deletions update-imported-docs/update-imported-docs

This file was deleted.

Loading