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

kubernetes/client-go:.github/PULL_REQUEST_TEMPLATE.md is unhelpful plus suggestion about SIG labelling #122238

Open
jsoref opened this issue Dec 8, 2023 · 19 comments · May be fixed by kubernetes/test-infra#31973
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. sig/testing Categorizes an issue or PR as relevant to SIG Testing.

Comments

@jsoref
Copy link
Contributor

jsoref commented Dec 8, 2023

What happened?

Drafting a PR to the kubernetes/client-go repo results in an unhelpful message with prose, but, no links, and the referenced content is a number of indirections away from anything remotely helpful.

What did you expect to happen?

The contents of the pr template should include links to relevant and helpful files. -- The README.md text should be a link to the master/HEAD file (this won't hurt someone who decides to edit the readme file in the repository or has already done so), and a reference to contributing should be a link to something that's useful and doesn't require following multiple links to get to an actual contributing file with useful information.

How can we reproduce it (as minimally and precisely as possible)?

  1. do the work to construct a code commit for a repository
  2. prepare to create a PR: https://github.com/kubernetes/client-go/compare/master...jsoref:client-go:issue-1321?expand=1
  3. read the message
  4. Note that README.md and CONTRIBUTING.md aren't links to anything which means you now have to go back x pages just to try to find them
  5. Assuming you've decided that the README.md isn't applicable, you now have to browse to (many steps, especially if you didn't start in the repository itself, which you won't in most cases, e.g. here, ...) and open CONTRIBUTING.md
  6. Read that text and realize that there was no need for the previous document to reference it -- the relevant text could have been inlined and the reference could have been to "the same contributing guide"
  7. Open https://git.k8s.io/kubernetes/CONTRIBUTING.md
  8. Read that even shorter text and realize that it too was a waste of reading (if you haven't gotten frustrated yet, you're doing way better than a newcomer, please let go of your prior knowledge, slow down, and recognize that all of these links and pages are actually frustrating)
  9. Get to "check out the Contributor's Guide" and sigh.
  10. Follow another link to another "Contributor's Guide" (https://github.com/kubernetes/community/tree/master/contributors/guide
  11. Note that this one randomly dumped you into a directory instead of an anything, so you'll probably have to scroll past all of the files to get to the readme -- if you haven't asked "why is this happening", you're using too much experience -- take a break, come back pretending that you don't know much about k8s and do understand that links can and should take you somewhere useful instead of dumping you into a gopher menu from 1991.
  12. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#readme
  13. Read some metadata
  14. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#getting-started
  15. Read a ToC
  16. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#contributor-guide
  17. follow the instructions in the Contributor Guide section which instruct you to read "this page" (which is not really where you are -- you're actually in a directory)
  18. Get annoyed at some of the prerequisites because they aren't really relevant -- and they also aren't even logical. A bot is not going to check to see if you have a GitHub account because you can't get to the bot until you have one.
  19. The link for a bot does not explain what it does because there is no k8s-ci-robot/k8s-ci-robot repo to show a pretty message explaining what the bot does -- which means if you click the link that is provided you'll be dumped somewhere useless.
  20. Assuming you were dumped somewhere useless and saw the pretty little graph of how busy this beaver is:
    image
  21. If you try really really hard and squint, you might find: https://github.com/kubernetes/test-infra/tree/master/prow#bots-home
  22. Click that link to https://github.com/kubernetes/test-infra/tree/master/prow#bots-home
  23. And get dumped in another gopher style directory listing,.
  24. Scroll down to the bottom of the window to see:
  25. "DOCUMENTATION DEPRECATION NOTICE: This file is deprecated."
  26. Sigh and go back a couple of frames -- your choose your own adventure is going to sap a lot of years from your life (you'd be better served watching The Princess Bride
  27. Now where were we? Lost in a maze of twisty passages, all alike?, ah, yes.
  28. We were trying to figure out how to contribute.
  29. Well, we've gotten to https://github.com/kubernetes/community/tree/master/contributors/guide#community-expectations-and-roles
  30. Go read Community Expectations -- it isn't helpful -- this stuff will not help you make an initial contribution and is not what you as a new contributor would expect from an item labeled "Community Expectations".
  31. See Community Membership -- this is also really not remotely helpful to someone looking to make an initial contribution.
  32. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#kubernetes-contributor-playground
  33. There's a Kubernetes Contributor Playground
  34. Follow the link.
  35. A Youtube playlist
  36. Note that YouTube is misspelled on this page as well as on the previous page -- apparently respect for other brands isn't a community value (this does appear to be the case, remember if you read https://github.com/kubernetes/community/blob/master/values.md#community-over-product-or-company that the "value" is "Community over product or company")
  37. You probably don't have time to watch the playlist (have you? do you?) -- I'm going to regret this, but, a quick check shows that it's probably well over a day log -- there are 6 items that are over an hour long each, and two that are 50+mins (and one that's nearly 50mins).
  38. Well, that was a good time sink. Let's go back and try to get to the end of that first guide -- remember you're already 2 links away from where you started and haven't learned anything about what you were trying to do (which is figure out what you actually need to do to make a PR -- which is probably understanding what labels you'll need to suggest to the bot or whether you can safely ignore the bot's insistence that you provide labels that you don't understand and can't determine on your own because you are in fact a new contributor).
  39. Ok, so there was a readme in https://github.com/kubernetes-sigs/contributor-playground/blob/master/README.md and it was confusing -- what's the "New Contributor Onboarding Track" and why isn't it a link to something?
  40. "A Youtube playlist of the New Contributor workshop has been posted, and an outline of content to videos can be found here." -- what does "has been posted" mean? has been posted when?
  41. Why wasn't remote a link to something?
  42. You leave the contributor playground with more questions than answers.
  43. If you do manage to find remote you'd discover it's from 2021 and has misbranded youtube differently (instead of Youtube)...
  44. If you really decided to visit https://github.com/kubernetes-sigs/contributor-playground/pulls you'd discover that area and sig and all the things you might need to actually understand to actually contribute to kubernetes/kubernetes are absent.
  45. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#contributor-workshops
  46. Is this another reference to the same youtube playlist as on the previous page? or not? do you dare click on it? I do not dare you to. I do not dare click on it.
  47. Scroll down to https://github.com/kubernetes/community/tree/master/contributors/guide#community
  48. There's a link to Community Membership Document is that the same as Community Membership from a few paragraphs above? Do you remember? If you cheat you'll discover it is, but you could ask "why is this link text different from that link text?" -- you should, but you're probably too exhausted by this point.
  49. Scroll to https://github.com/kubernetes/community/tree/master/contributors/guide#communication
  50. Follow the link to https://github.com/kubernetes/community/tree/master/communication? I wouldn't recommend it. Amusingly they spelled YouTube correctly. Unsurprisingly they did not write Zoom correctly.
  51. There's a link to Kubernetes code of conduct (which is not written with the proper case -- so you don't know if it's the same as other documents or not, it is, in fact, the whole text is silly as it's actually the CNCF CoC -- I believe that this issue falls well within that guidance).
  52. Scroll to https://github.com/kubernetes/community/tree/master/contributors/guide#events and decide that the rest of the items here are not helpful.
  53. Scroll to https://github.com/kubernetes/community/tree/master/contributors/guide#advanced-topics
  54. "This section includes things that need to be documented, but typical contributors do not need to interact with regularly." -- great, this is clearly not something that you're supposed to read, it says so right there! -- They're right, even though one might hope that SIGs/AREAs/... are related to ownership, the referenced documentation will not help you as a new contributor figure out what to do.
  55. There are other files in the directory. At this point it's time to give up. You recall somewhere that some page mentioned Slack -- I'd recommend trying to figure out how to get to slack and then getting lost trying to figure out which slack channel is appropriate for asking for help filing this report. -- This report filed at the behest of someone in a slack channel -- I would not have spent the time to do so without being asked to do so, this was a very expensive ticket.

Anything else we need to know?

https://github.com/kubernetes/client-go/blob/master/.github/PULL_REQUEST_TEMPLATE.md

I understand that you don't want to maintain the repository itself, but by the time a user sees this message, they've already tried very hard to compose a PR, they shouldn't be forced to do many jumps of indirection and jump through many hoops to get to a place that tells them where to go.

If retaining a reference to CONTRIBUTING.md in the kubernetes/client-go repo is deemed essential, it should really be a link to https://github.com/kubernetes/client-go/blob/HEAD/CONTRIBUTING.md (if you're afraid that someday master will be replaced by main...) instead of just words -- if a user already tried to do a thing, giving them a message that requires them to take x steps instead of a path forward is really frustrating.
And that CONTRIBUTING.md confusingly (it has more words than the thing it) points to https://github.com/kubernetes/kubernetes/blob/master/CONTRIBUTING.md
which then points to another contributing guide: https://github.com/kubernetes/community/tree/master/contributors/guide

Kubernetes version

N/A

Cloud provider

N/A

OS version

Web submission

Install tools

No response

Container runtime (CRI) and version (if applicable)

No response

Related plugins (CNI, CSI, ...) and versions (if applicable)

No response

@jsoref jsoref added the kind/bug Categorizes issue or PR as related to a bug. label Dec 8, 2023
@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Dec 8, 2023
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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 k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 8, 2023
@jsoref
Copy link
Contributor Author

jsoref commented Dec 8, 2023

/sig Contributor Experience

@k8s-ci-robot
Copy link
Contributor

@jsoref: The label(s) sig/contributor, sig/experience cannot be applied, because the repository doesn't have them.

In response to this:

/sig Contributor Experience

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.

@jsoref
Copy link
Contributor Author

jsoref commented Dec 8, 2023

Please note that the bot @k8s-ci-robot said /sig <group-name> and https://github.com/kubernetes/community/blob/master/sig-list.md#special-interest-groups clearly says that the Name is Contributor Experience:

Name Label Chairs Contact Meetings
API Machinery api-machinery * David Eads, Red Hat* Federico Bongiovanni, Google * Slack* Mailing List * Kubebuilder Meeting: Thursdays at 11:00 PT (Pacific Time) (biweekly)* Regular SIG Meeting: Wednesdays at 11:00 PT (Pacific Time) (biweekly)
Contributor Experience contributor-experience * Kaslin Fields, Google* Nabarun Pal, VMware * Slack* Mailing List * Regular SIG Meeting: Wednesdays at 9:00 PT (Pacific Time) (weekly alternating slack/zoom)* (contributor-comms) Contributor Comms - Contributor Comms Team Meeting: Fridays at 8:00 PT (Pacific Time) (weekly)* (github-management) GitHub Administration Subproject: Thursdays at 09:00 PT (Pacific Time) (Monthly on 4th Thursday)

@jsoref
Copy link
Contributor Author

jsoref commented Dec 8, 2023

/sig contributor-experience

@k8s-ci-robot k8s-ci-robot added sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Dec 8, 2023
@jsoref
Copy link
Contributor Author

jsoref commented Dec 8, 2023

@k8s-ci-robot 's help should say /sig <group-label> not /sig <group-name>

@sftim
Copy link
Contributor

sftim commented Dec 9, 2023

/kind feature

The existing template provides some help; this is a request to make it even more helpful. I presume we could repeat that for lots of other automatically managed Git repositories.
/remove-kind bug

Thank you for the suggestion.

@k8s-ci-robot k8s-ci-robot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/bug Categorizes issue or PR as related to a bug. labels Dec 9, 2023
@sftim
Copy link
Contributor

sftim commented Dec 9, 2023

@k8s-ci-robot 's help should say /sig <group-label> not /sig <group-name>

Not quite. /sig sig/contributor-experience would also be wrong. However, if we find a volunteer to do it, we could accept the alternatives when they are unambiguous and provide better advice when they are not.

The SIG that looks after this automation is SIG Testing.
/sig testing

@k8s-ci-robot k8s-ci-robot added the sig/testing Categorizes an issue or PR as relevant to SIG Testing. label Dec 9, 2023
@sftim
Copy link
Contributor

sftim commented Dec 9, 2023

/retitle kubernetes/client-go:.github/PULL_REQUEST_TEMPLATE.md is unhelpful plus suggestion about SIG labelling

@k8s-ci-robot k8s-ci-robot changed the title kubernetes/client-go:.github/PULL_REQUEST_TEMPLATE.md is unhelpful kubernetes/client-go:.github/PULL_REQUEST_TEMPLATE.md is unhelpful plus suggestion about SIG labelling Dec 9, 2023
@jsoref
Copy link
Contributor Author

jsoref commented Dec 10, 2023

@sftim: wait, the Column Title clearly says Label, it sounds like you're saying that's wrong and it should in fact say Label Suffix and then the help could be changed to /sig <sig-label-suffix>?

@jsoref
Copy link
Contributor Author

jsoref commented Dec 10, 2023

Note: I'm willing to do this work if people can point to the places and offer to support me doing the work.

@BenTheElder
Copy link
Member

This seems like at least a dozen different issues filed as one enormous issue which makes it difficult to actionably fix them.

I appreciate that these things are painful and writing them up to highlight them is potentially useful but as only one large issue they're difficult to follow up on, remember that many responding to these are also likely seeing an influx of issues.

SIG Testing runs and implements the automation, but currently Contributor Experience maintains that all contributor facing changes to the behavior must be approved by them, including changes to the help text ...

The robot currently grew out in https://github.com/kubernetes/test-infra (though we're hopefully about to break it out into a distinct kubernetes-sigs/prow repo ...). The prow/ directory contains most of the relevant bits, along with config/ which contains the config for our specific instance (some of the comments are configurable templates).

I can't offer much bandwidth myself at the moment, there's too many things to take care of. Usability papercuts pile up but the thing is no one is paying people to work on the robots much anymore and volunteers are pretty busy trying to keep things running (e.g. kubernetes/k8s.io#6165).

Some of the maze of links is because there's a generic template project with stub files, and client-go just has the stub files. They might accept adding more details, IMHO it would be good for the staging repos to have their own more-specific CONTRIBUTING.md contents that contain the details on how to contribute to these. The template repo stub files explicitly suggest that subprojects add their own details but that hasn't happened here yet.

@BenTheElder
Copy link
Member

Also comments like:

Note that YouTube is misspelled on this page as well as on the previous page -- apparently respect for other brands isn't a community value (this does appear to be the case, remember if you read https://github.com/kubernetes/community/blob/master/values.md#community-over-product-or-company that the "value" is "Community over product or company")

Are a bit over the top. Mistakes happen, missing the case on this isn't any contributor intentionally disrespecting anyone or any company.

As for the values reference: The point of "Community over product or company" is that we strive to make decisions and take actions on the basis of what's best the project at large rather than favoring a particular company including our own employers, and the project on the whole aims for vendor-neutrality.

It's impolite not to assume best intentions.
I understand you were frustrated when writing this issue, but let's remember that our contributors and maintainers are doing their best.

These docs could use more contributors cleaning them up.

Get annoyed at some of the prerequisites because they aren't really relevant -- and they also aren't even logical. A bot is not going to check to see if you have a GitHub account because you can't get to the bot until you have one.

The contributor guide is general to anyone looking to contribute, the bot links to this rather than duplicating most of this content somewhere else.

We should probably start linking to https://www.kubernetes.dev/docs/guide/ instead of github. I'm not sure if Contributor Experience considers this replacement ready though, there was a period where there were two copies of the contributing docs with one intended for this site and one the legacy content ...

There's a link to Kubernetes code of conduct (which is not written with the proper case -- so you don't know if it's the same as other documents or not, it is, in fact, the whole text is silly as it's actually the CNCF CoC -- I believe that this issue falls well within that guidance).

GitHub looks for a code-of-conduct.md and marks if a repo has one or not.
The current policy is to defer to CNCF's Code of Conduct.

@jsoref
Copy link
Contributor Author

jsoref commented Dec 14, 2023

Sorry. For pretty much every item I enumerated, I'm willing to do the work to improve the experience (as long as people are willing to help me deal with whatever hurdles I might encounter, whether that's policy, bots, quirks, or just a lack of an obvious path forward). I happen to have a tool that works on spelling things (including branding items like YouTube) and thus that's actually the easiest item for me to fix.

But yes, this is absolutely death by a thousand paper-cuts. I understand you're low on volunteers, but if the hurdle to get through the door is sufficiently high, you won't get many more and you'll continue to bleed or hemorrhage as applicable.

Anyway, pick 5 of the paper-cuts identified above that you (the project, not the person) are willing to let me improve, suggest a general direction that would be acceptable to the project and I'll see what I can do.

One approach I've found that works well for things like this is for me to "sit" (virtually or physically) with someone and go over a section asking questions, clarifying, and fixing things. It's usually possible to get some portion fixed in 15 minutes, although longer runs can take half an hour or an hour, but conveniently whatever is addressed is a clear improvement and it's an interruptible/resumable process as it's unlikely that anyone else will be changing the process or the documentation between interrupting and resuming.

I haven't even gotten to the paper-cuts involved in actually trying to make a contribution for this project.
The PR template includes a link to:

  • I'm glad it tells me not to create a PR based on running a spell-checker across the whole repository.

    You can look at the OWNERS files in a directory (or its parent directory) to see who owns that code, and then group the changes together accordingly (e.g., with one PR touching files in cmd/kube-proxy and pkg/util/iptables, which are owned by SIG Network, and another PR touching files in pkg/kubelet and pkg/controller/nodelifecycle, which are owned by SIG Node.)

    Sadly, that . at the end is on the wrong side of the line. It seems like the first thing I'd want to work on is just the PR template itself.

  • Read up on GoDoc - follow those general rules for comments.

    • for my future reference, it appears to be Godoc ...
    • note that the blog has a note about an updated page and I can't tell if the blog or the updated thing is really what one should be reading...
  • https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#trivial-edits

    If you find one grammatical or spelling error, it is likely there are more in that file, you can really make your Pull Request count by checking the formatting, checking for broken links, and fixing errors and then submitting all the fixes at once to that file.

    So, it seems that at least outlining the flaws for a section as opposed to trying to do a single item is recommended by this file...

    Note: I have no intention of boiling any oceans. I also have lots of experience splitting work into reviewable chunks.

    A generic explanation of how labels are used in pull requests can be found here.

    • here links are generally considered harmful (the same critique would apply to "See the current list of tests at this link")
    • (the owners.md file could also use love, but in the spirit of not boiling oceans, let's focus on fixing the PR template first...)

    The behavior of Prow is configurable across projects. You should be aware of the following configurable behaviors.

    • That . should really be a : (there was a colon used a few lines up...)

    If the failure is a flake, anyone can comment /retest if the pull request is trusted

    • This item is repeated from somewhere else in the flow -- by the time I'm here, I've lost all the useful context, and I don't think it's possible for the PR not to be trusted, which to me makes it seem like at least if the pull request is trusted is superfluous/counterproductive.
  • https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#more-about-ok-to-test

    The ok-to-test label is applied by org members to PRs from external contributors, it signals that the PR can be tested.

    • the earlier ok-to-test text used the word trusted, this section doesn't, that's unfortunate because it means that the two sections aren't well linked.

    An ok-to-test label may reduce the workload and smoothens the contributors experience as they can know if there is any failing test. If there is, you can fix the test and they don't have to wait for a long time to get a review from maintainer/assignee.

    • I can't tell who you is here -- is it the reviewer or the contributor?

    PRs with tag do-not-merge/hold or needs-rebase should make the appropriate changes before the PR can be labelled ok-to-test.

    • voice got worse here, at least before there was a person, not the PR itself that needs to make changes? 😵 -- it's probably as simple as changing make to have and before the PR can be labelled ok-to-test to made before a reviewer (?) labels the PR with/as ok-to-test

    PRs created by mistake without to meaningful change of code should not be labelled ok-to-test and closed.

    The not + and here doesn't actually do what the author meant -- and closed should probably be changed to but be closed instead.

@thockin
Copy link
Member

thockin commented Feb 13, 2024

None of the staging-derived repos actually take PRs (the doc says "Please send pull requests against the client packages in the Kubernetes main repository. Changes in the staging area will be published to this repository every day.")

Yeah, many of these issues suck, I agree, but really you should not be able to open a PR against client-go at all, right? Can we just turn that off and emphasize the "PRs go to k/k/staging/k8s.io/client-go" instead?

@kubernetes/owners

@BenTheElder
Copy link
Member

BenTheElder commented Feb 13, 2024

Yeah, many of these issues suck, I agree, but really you should not be able to open a PR against client-go at all, right? Can we just turn that off and emphasize the "PRs go to k/k/staging/k8s.io/client-go" instead?

GitHub allows disabling issues for a repo but not pull requests.

We could add another bot behavior to auto-close PRs to these. Not great.

@jsoref
Copy link
Contributor Author

jsoref commented Feb 13, 2024

Fwiw, GitHub itself has the ability to block PRs:

Try loading: github/gh-actions-importer@main...add-tests-for-executable

An owner of this repository has limited the ability to open a pull request to users that have contributed to this repository in the past.

See /settings/interaction_limits

You'd need to renew it every 6 months (I'd suggest renewing it every 5 months).

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 13, 2024
@jsoref
Copy link
Contributor Author

jsoref commented May 13, 2024

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. sig/testing Categorizes an issue or PR as relevant to SIG Testing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants