feat(owlbot-bootstrapper): get full mono repo path, mono repo name, and mono repo org from the cli args#4577
Merged
Conversation
chingor13
requested changes
Nov 1, 2022
| throw new Error('No language-specific container specified'); | ||
| } | ||
|
|
||
| export function getMonoRepoNameAndOrg(repoToClone: string | undefined) { |
Contributor
There was a problem hiding this comment.
Please define the return type explicitly
Contributor
There was a problem hiding this comment.
Also, this is probably better named as parseRepoNameAndOrg or parseGitHubRepository as the fact that it's a mono-repo is irrelevant.
| export async function runTrigger( | ||
| argv: CliArgs, | ||
| cb: CloudBuildClient, | ||
| monoRepoNameAndOrg: {monoRepoName: string; monoRepoOrg: string}, |
Contributor
There was a problem hiding this comment.
This type could be re-used. Additionally, the monoRepo part could be redundant.
Consider a type like:
interface GitHubRepository {
owner: string;
name: string;
}
Comment on lines
+166
to
+191
| it('gets the correct mono repo name, or throws an error', () => { | ||
| assert.deepStrictEqual( | ||
| runTriggerCommand.getMonoRepoNameAndOrg( | ||
| 'git@github.com:googleapis/google-cloud-node.git' | ||
| ), | ||
| {monoRepoOrg: 'googleapis', monoRepoName: 'google-cloud-node'} | ||
| ); | ||
|
|
||
| assert.deepStrictEqual( | ||
| runTriggerCommand.getMonoRepoNameAndOrg( | ||
| 'git@github.com/googleapis/google-cloud-node.git' | ||
| ), | ||
| {monoRepoOrg: 'googleapis', monoRepoName: 'google-cloud-node'} | ||
| ); | ||
|
|
||
| assert.deepStrictEqual( | ||
| runTriggerCommand.getMonoRepoNameAndOrg( | ||
| 'git@github.com:googleapis/nodejs-scheduler.git' | ||
| ), | ||
| {monoRepoOrg: 'googleapis', monoRepoName: 'nodejs-scheduler'} | ||
| ); | ||
|
|
||
| assert.throws(() => { | ||
| runTriggerCommand.getMonoRepoNameAndOrg(undefined); | ||
| }, new Error("Repo to clone arg is malformed; should be in form of ssh address,' git@github.com:googleapis/google-cloud-node.git'")); | ||
| }); |
Contributor
There was a problem hiding this comment.
nit: put these in a separate describe('getMonoRepoNameAndOrg') block
chingor13
approved these changes
Nov 1, 2022
| ); | ||
| }); | ||
|
|
||
| describe('get mono repo name and org from ssh address', () => { |
Contributor
There was a problem hiding this comment.
nit: I would just name this describe block the exported function name being tested
This lets you easily run the subset of tests:
npm t -- -g 'parseRepoNameAndOrg'
Closed
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds in an argument to the owlbot-bootstrapper, the full mono-repo path, prompted by:
Should this include the repo name so I can `cd "${MONO_REPO_PATH}"` in my container? Otherwise, to be re-usable, you need to parse the `REPO_TO_CLONE` env to grab the repo name and then cd into that parsed value (or hard-code the repo name into the container).Since the CLI should only receive the directory in which to clone the mono repo, and the ssh address of the mono repo to clone, this PR does the following things:
Parses the repoToClone ssh address to get the mono repo name; then concatenates it to the monoRepoDir, which is the actual cli input. Then, throughout the rest of the process, we pass through the monoRepoDir, and the monoRepoPath. Language containers will only see the monoRepoPath.
Since we're already parsing the repoToClone ssh address, I'm removing where we do that in the common container, and instead passing through the parsed variables (monoRepoName and monoRepoOrg). This standardizes the regex matching we're doing as well.
Instructions updated in cl/481195940