-
Notifications
You must be signed in to change notification settings - Fork 32
docs: consolidate onboarding instructions #2804
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
base: main
Are you sure you want to change the base?
Conversation
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively consolidates the onboarding instructions by moving the prerequisites section from doc/library-maintainer-guide.md to doc/onboarding.md. This change improves the structure and clarity of the documentation. I have one suggestion to improve the user experience for new contributors.
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively consolidates the onboarding and development environment setup instructions by moving the prerequisites from library-maintainer-guide.md to onboarding.md. This is a good refactoring that improves the documentation structure and reduces duplication. I have one suggestion to improve the clarity of the prerequisites for new contributors.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2804 +/- ##
=======================================
Coverage 86.50% 86.50%
=======================================
Files 120 120
Lines 10232 10232
=======================================
Hits 8851 8851
Misses 977 977
Partials 404 404 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| ## Step 2: Set Up Your Editor | ||
| - Linux | ||
| - [Go](https://go.dev/doc/install) | ||
| - sudoless Docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - sudoless Docker | |
| - [sudoless Docker] (go/docker) |
| - [Go](https://go.dev/doc/install) | ||
| - sudoless Docker | ||
| - git (if you wish to build it locally) | ||
| - gcloud (to set up Docker access to conatiner images) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - gcloud (to set up Docker access to conatiner images) | |
| - [gcloud](https://g3doc.corp.google.com/company/teams/cloud-sdk/cli/index.md?cl=head#installing-and-using-the-cloud-sdk) (to set up Docker access to conatiner images) |
| - gcloud (to set up Docker access to conatiner images) | ||
| - [gh](https://github.com/cli/cli) for GitHub access tokens | ||
|
|
||
| While in theory `librarian` can be run in non-Linux environments that support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| While in theory `librarian` can be run in non-Linux environments that support | |
| `librarian` should be run from your remote desktop |
| - [gh](https://github.com/cli/cli) for GitHub access tokens | ||
|
|
||
| While in theory `librarian` can be run in non-Linux environments that support | ||
| Linux Docker containers, Google policies make this at least somewhat infeasible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Linux Docker containers, Google policies make this at least somewhat infeasible |
|
|
||
| While in theory `librarian` can be run in non-Linux environments that support | ||
| Linux Docker containers, Google policies make this at least somewhat infeasible | ||
| (while staying conformant), so `librarian` is not tested other than on Linux. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| (while staying conformant), so `librarian` is not tested other than on Linux. |
| > practical way of abstracting away language details. We are confident that | ||
| > there are enough Googlers who require Docker to work on gLinux that it won't | ||
| > actually go away any time soon. We may investigate using podman instead if | ||
| > necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| > necessary. |
| project and begin contributing effectively. | ||
|
|
||
| ## Step 1: Install Go | ||
| ## Prerequisites |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Prerequisites | |
| ## Step 1: Setup Environment to Run Librarian |
| These will teach you the foundations for how to write, run, and test Go code. | ||
|
|
||
| ## Step 5: Understand How We Write Go | ||
| ## Step 4: Understand How We Write Go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's Add another step:
6: Running Librarian
This should pull in the following 3 sections (combined as one section):
- https://github.com/googleapis/librarian/blob/main/doc/library-maintainer-guide.md#running-librarian
- https://github.com/googleapis/librarian/blob/main/doc/library-maintainer-guide.md#obtaining-a-github-access-token
- https://github.com/googleapis/librarian/blob/main/doc/library-maintainer-guide.md#repository-and-library-options
| ```sh | ||
| gcloud auth configure-docker us-central1-docker.pkg.dev | ||
| ``` | ||
| See [onboarding](onboarding.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See [Setup Environment to Run Librarian](link directly to section)
| ``` | ||
| See [onboarding](onboarding.md). | ||
|
|
||
| ## Running `librarian` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace this section with:
See [Running Librarian](link directly to section in onboarding doc)
Fixes #2721