migrate site content: contributing docs #4709
Conversation
|
@dani-santos-code: This PR has multiple commits, and the default merge method is: merge. 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. |
k8s-ci-robot
added
the
cncf-cla: no
|
Welcome @dani-santos-code! |
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @dani-santos-code. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
k8s-ci-robot
added
the
size/L
k8s-ci-robot
added
cncf-cla: yes
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
@KnVerey Would it be possible to set up a netlify preview in CI like we have in cli-experimental here? |
I want to make sure we don't publish the site "for real" anywhere before it is ready, but you're right that the preview would be very handy. I've put in a request at kubernetes/org#3564. If it is approved, we will need to figure out how to disable deploys off the master branch (or failing that, disable crawlers). |
dani-santos-code
force-pushed
the
ds/migrate-contributing-docs
branch
from
6778953
to
cb402f5
Compare
| {{< /alert >}} | ||
|
|
||
| ## Call stack | ||
| Call stack when running `kustomize build`, with links to code: |
There was a problem hiding this comment.
this part changed quite a bit. I updated all the links, but since the code changed quite a bit, I'd like some confirmation on what to display here. Also, not quite sure if we want to capture every single call...
for reference, this is what we had before: https://kubectl.docs.kubernetes.io/contributing/kustomize/howitworks/
alternatively, we can just mention the main functions: Build Command(the entry point), MakeKustomizer, Run, and the bit that writes the output to a yaml file.
what do you think? I'm not sure it is so helpful linking to every single func and the additional descriptions, since we can read them once we navigate through the code.
Once one identifies the entry point and the main functions, it's easy to navigate on Github, by clicking through. I wouldn't go back to this doc in order to do that.
At least, I found it a bit confusing when following the instructions
There was a problem hiding this comment.
I agree with you @dani-santos-code -- it's more important to identify the key entrypoints for exploration than to walk through every bit, which will get stale, and which probably isn't a good experience for the reader anyway. As a new contributor, you're really the target audience here! So feel free to rewrite this page to reflect the portion you find interesting/useful. 😄
There was a problem hiding this comment.
addressed here: b42597e. let me know if this rewording is ok
|
thank you for your feedback @natasha41575. I think I addressed most of your feedback. Would appreciate a re-review. Thank you! 🙏 |
dani-santos-code
force-pushed
the
ds/migrate-contributing-docs
branch
from
cb402f5
to
809d648
Compare
| [kind/feature]: https://github.com/kubernetes-sigs/kustomize/labels/kind%2Ffeature | ||
| [sig-cli bi-weekly]: https://github.com/kubernetes/community/tree/master/sig-cli#meetings | ||
|
|
||
| Following is the process for proposing a new Kustomize feature: |
There was a problem hiding this comment.
We have instructions about this in three places:
- here / the old docs
- https://github.com/kubernetes-sigs/kustomize/tree/master/proposals
- the issue template itself
I've added a note to the new coordination spreadsheet about this. I'm thinking 1+2 should move to the issue template, the rest should be integrated with https://github.com/kubernetes-sigs/kustomize/tree/master/proposals where missing, and this page should just point there. If you want to add that to this PR then go for it, but otherwise we can defer it. At minimum, let's remove step 3 (which we don't want them to do in the general case), step 7 (not really relevant IMO), and the "Expect comments..." line, which isn't realistic, as much as I wish it were.
There was a problem hiding this comment.
oh, okie! makes sense!
I think just adding to the docs you shared makes sense since it already outlines what one should do. And yeah, I went through the issue template. It adds the label and everything.
So opting to just point to the proposals doc. Addressed here:
eeb237d
| {{< /alert >}} | ||
|
|
||
| ## Call stack | ||
| Call stack when running `kustomize build`, with links to code: |
There was a problem hiding this comment.
I agree with you @dani-santos-code -- it's more important to identify the key entrypoints for exploration than to walk through every bit, which will get stale, and which probably isn't a good experience for the reader anyway. As a new contributor, you're really the target audience here! So feel free to rewrite this page to reflect the portion you find interesting/useful. 😄
|
|
||
| Add `go` to your PATH | ||
|
|
||
| ### Install kubeval |
There was a problem hiding this comment.
Let's change this to just Install tools and have them run make install-tools, so we don't have to keep these lists up to date (currently incomplete).
There was a problem hiding this comment.
updated here: 324de9c
| - Navigate to the Kustomize `travis` directory | ||
| - ```Example: C:\_go\src\sigs.k8s.io\kustomize\travis``` | ||
| - Now Execute: | ||
| - ```.\Invoke-PreCommit.ps1``` |
There was a problem hiding this comment.
@natasha41575 wdyt about deleting the Windows doc and the script referenced here entirely? I can't test it, but at a glance I suspect that what this will run is very incomplete compared to the make verify-kustomize-repo target we use. And e.g. the Travis reference above is definitely long-invalid. As much as I'd love to have a Windows contributor, a doc we can't/don't effectively maintain probably won't be helpful in any case.
There was a problem hiding this comment.
Let's go ahead and remove this.
There was a problem hiding this comment.
removed here: b32b355
Co-authored-by: Katrina Verey <kn.verey@gmail.com>
Co-authored-by: Katrina Verey <kn.verey@gmail.com>
Co-authored-by: Katrina Verey <kn.verey@gmail.com>
Co-authored-by: Katrina Verey <kn.verey@gmail.com>
Co-authored-by: Katrina Verey <kn.verey@gmail.com>
dani-santos-code
force-pushed
the
ds/migrate-contributing-docs
branch
from
361cf72
to
eeb237d
Compare
|
/tide merge-method-squash |
|
/label tide/merge-method-squash |
k8s-ci-robot
added
the
tide/merge-method-squash
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dani-santos-code, KnVerey 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:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
This PR is part of the larger docs consolidation issue: #4338
I copied over the files from cli-experimental, ran locally and it seems to be correct
I followed a similar pattern we see in this other migration PR