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

Go workspaces blog post #45358

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Go workspaces blog post #45358

merged 1 commit into from
Mar 20, 2024

Conversation

thockin
Copy link
Member

@thockin thockin commented Feb 26, 2024

A blog post on Go workspaces in k/k

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. area/blog Issues or PRs related to the Kubernetes Blog subproject language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Feb 26, 2024
Copy link

netlify bot commented Feb 26, 2024

Pull request preview available for checking

Built without sensitive environment variables

Name Link
🔨 Latest commit 281b42f
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-io-main-staging/deploys/65f80e8948998100082e88c3
😎 Deploy Preview https://deploy-preview-45358--kubernetes-io-main-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sftim
Copy link
Contributor

sftim commented Feb 27, 2024

How about making this an article on https://k8s.dev/blog/

@thockin
Copy link
Member Author

thockin commented Feb 28, 2024

Instead of this? I don't know the mechanics of the blog-publishing pipeline :)

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 9, 2024
@sftim
Copy link
Contributor

sftim commented Mar 9, 2024

Instead of this? I don't know the mechanics of the blog-publishing pipeline :)

Keep this PR, and use it to make a mirror copy of the canonical article.

@sftim
Copy link
Contributor

sftim commented Mar 9, 2024

I hope to get this documented (by which I specifically mean: I hope to coach someone else to write it up).

@thockin
Copy link
Member Author

thockin commented Mar 9, 2024

I have a very first draft up. I don't know how intersting it will be to non-contributors, but feedback is very welcome.

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 12, 2024
@k8s-ci-robot k8s-ci-robot requested a review from aojea March 12, 2024 01:22
Copy link
Contributor

@natalisucks natalisucks left a comment

Choose a reason for hiding this comment

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

I've done a sweep of this for grammar/punctuation and did some suggesting for that, I hope that's alright in terms of review etiquette. I've also added a couple of comments, so feel free to address those when you get the chance.

@natalisucks What I could do most with is a high-level feeling - is it on the right track? Is it vaguely interesting? Is it too long or too short? Are there "beats" I need to hit that are missing? Do I need to throw it away and start over?

I think this article does really well to spell out how you/the project has dealt with some technical debt and that even big projects like Kubernetes can benefit from often "thankless" work. My one ask would be to sort of weave in a call to action for folks to get more involved, since we always need some extra hands. You definitely don't need to throw this away or start over 😸

content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
content/en/blog/_posts/2024-03-19-go-workspaces.md Outdated Show resolved Hide resolved
@sftim
Copy link
Contributor

sftim commented Mar 14, 2024

/hold

pending assignment of publication date

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 14, 2024
---
layout: blog
title: 'Using Go workspaces in Kubernetes'
date: 2024-03-19
Copy link
Contributor

Choose a reason for hiding this comment

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

@thockin: When would you like this published? Is there a particular time during the day that is good for publication?

Copy link
Member Author

Choose a reason for hiding this comment

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

No particular.

@thockin
Copy link
Member Author

thockin commented Mar 15, 2024

Re-pushed with all comments addressed (I think)

@natalisucks
Copy link
Contributor

@sftim Regarding publication date, I'll check in with our Release Comms lead on how she'd like to start organizing the feature blogs for this release and get back to you. As a note, we have nine feature blogs in the editing pipeline currently, and are in the midst of writing the release blog as well.

This feature blog has had technical review (aside from the author itself), and now Release Comms review. If we're comfortable to call this ready to publish (barring the decided date) @sftim, let me know, or feel free to do another review as you wish.

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 15, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 8378b185bf7ee4fbedbe797927b63c5bb9516754

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

I'll make a branch of https://github.com/kubernetes/contributor-site with my suggestions already in, and I'll see about getting that reviewed.

deal with a certain amount of constant low-grade pain. Our custom `staging`
mechanism let us bend the rules of Go; it was a little clunky, but when it
worked (which was most of the time) it worked pretty well. When it failed, the
errors were inscrutable and un-Googleable -- nobody else was doing the silly
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit) I'd use a real em dash:

Suggested change
errors were inscrutable and un-Googleable -- nobody else was doing the silly
errors were inscrutable and un-Googleable nobody else was doing the silly

mechanism let us bend the rules of Go; it was a little clunky, but when it
worked (which was most of the time) it worked pretty well. When it failed, the
errors were inscrutable and un-Googleable -- nobody else was doing the silly
things we were doing. Usually the fix was to re-run one or more of the update-*
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit

Suggested change
things we were doing. Usually the fix was to re-run one or more of the update-*
things we were doing. Usually the fix was to re-run one or more of the `update-*`

Comment on lines 80 to 85
1) Most of the time it only hurt a little (punctuated with short moments of
more acute pain).
2) Kubernetes was still growing in popularity - we all had other, more urgent
things to work on.
3) The fix was not obvious, and whatever we came up with was going to be both
hard and tedious.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, a request: please use a Markdown ordered list here.


Along the way, the Go language team saw what we (and others) were doing and
didn't love it. They designed a new way of stitching multiple modules together
into a new "workspace" concept. Once enrolled in a workspace, Go tools had
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
into a new "workspace" concept. Once enrolled in a workspace, Go tools had
into a new _workspace_ concept. Once enrolled in a workspace, Go tools had

didn't love it. They designed a new way of stitching multiple modules together
into a new "workspace" concept. Once enrolled in a workspace, Go tools had
enough information to work in any directory structure and across modules,
without GOPATH or symlinks or other dirty tricks.
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit)

Suggested change
without GOPATH or symlinks or other dirty tricks.
without `GOPATH` or symlinks or other dirty tricks.

Comment on lines 116 to 118
1) Those parsing libraries didn't understand modules or workspaces.
2) GOPATH allowed us to pretend that Go "package paths" and directories on
disk were interchangeable in trivial ways. They are not.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1) Those parsing libraries didn't understand modules or workspaces.
2) GOPATH allowed us to pretend that Go "package paths" and directories on
disk were interchangeable in trivial ways. They are not.
1. Those parsing libraries didn't understand modules or workspaces.
2. `GOPATH` allowed us to pretend that Go _package paths_ and directories on
disk were interchangeable in trivial ways. They are not.


One unfortunate result of this work was that I had to break compatibility. The
gengo library simply did not have enough information to process packages
outside of GOPATH. After discussion with gengo and Kubernetes maintainers, we
Copy link
Contributor

Choose a reason for hiding this comment

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

(nit)

Suggested change
outside of GOPATH. After discussion with gengo and Kubernetes maintainers, we
outside of `GOPATH`. After discussion with gengo and Kubernetes maintainers, we

@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 18, 2024
@sftim
Copy link
Contributor

sftim commented Mar 18, 2024

This mirrors kubernetes/contributor-site#484

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 18, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: fa3cb7f053864d2b54fa44fa0b9d1fcd1c9b7294

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sftim

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2024
@sftim
Copy link
Contributor

sftim commented Mar 20, 2024

D'oh - forgot to unhold this one.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 20, 2024
@k8s-ci-robot k8s-ci-robot merged commit de07f19 into kubernetes:main Mar 20, 2024
6 checks passed
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. area/blog Issues or PRs related to the Kubernetes Blog subproject cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Published
Development

Successfully merging this pull request may close these issues.

7 participants