Adding mini CLI script for cloning repos by quaid · Pull Request #40 · operate-first/toolbox

quaid · 2021-07-13T22:51:36Z

Not finding a similar tool and tired already of forgetting to do my pre-commit install step when cloning a repo, >
It's in a repo so it can be maintained, moved around, etc.
When I asked in Slack #general if such a tool existed, I found out it didn't and it was suggested (thanks @tumido f>
IIUC, my patch to the Dockerfile should pull in v0.1 of this bash script, drop it where it's in the user's $PATH, and makes sure the file is executable
Patches to this patch, and to the upstream script, are very welcome
Also, suggestions on what I can do better in v0.2 are welcome; my bash skills are basic and rusty, happy for any help and tips
My concept here is that we may want to have a handful of individual tools, and then be able call them by o1 toolname OPTIONS ARGUMENT TARGET-sort of thing, as a way to ease the life of contributors, and help keep our work net and tidy. :)

Signed-of by: Karsten Wade [email protected] [email protected]

Related Issues and Dependencies

None

This introduces a breaking change

Yes
No

(It shouldn't! but I didn't test the Dockerfile)

This Pull Request implements

Adding a mini CLI tool for more easily cloning Operate First project repositories, then setting up the proper environment for the newly cloned repo.

Currently, it takes a repo name, clones it, and runs pre-commit install to install pre-commit hooks.

o1-clone REPONAME

This script can be extended for any common newly cloned repo actions

Description

Adding the variable, Dockerfile description element, and call to download and include the specified version of the tool (https://github.com/quaid/o1-tools/releases/tag/v0.1) in the resulting Toolbox build.

@tumido

- Not finding a similar tool and tired already of forgetting to do my `pre-commit install` step when cloning a repo, > - It's in a repo so it can be maintained, moved around, etc. - When I asked in Slack #general if such a tool existed, I found out it didn't and it was suggested (thanks @tumido f> - IIUC, my patch to the Dockerfile should pull in v0.1 of this bash script, drop it where it's in the user's $PATH, and makes sure the file is executable - Patches to this patch, and to the upstream script, are very welcome - Also, suggestions on what I can do better in v0.2 are welcome; my bash skills are basic and rusty, happy for any help and tips - My concept here is that we may want to have a handful of individual tools, and then be able call them by `o1 toolname OPTIONS ARGUMENT TARGET`-sort of thing, as a way to ease the life of contributors, and help keep our work net and tidy. :) Signed-of by: Karsten Wade <[email protected]> <[email protected]>

- I'm new at this Dockerfile thing, and I realized when viewing the PR online what I had missed. - Live and learn :) Signed-off by: Karsten Wade <[email protected]> <[email protected]>

tumido · 2021-07-15T11:14:28Z

@quaid hm.. I'm thinking.. is there a reason to keep that script in a separate repository? What about we include it here directly into the scripts/ directory?

quaid · 2021-07-15T23:50:37Z

/retest

quaid · 2021-07-16T02:17:15Z

@tumido:

@quaid hm.. I'm thinking.. is there a reason to keep that script in a separate repository? What about we include it here directly into the scripts/ directory?

As a concept, I would prefer this script in the upstream project, yes. I can then switch development to a fork of this repo, which sounds easier in the long run. 😁

Would we instead point the curl call at https://raw.githubusercontent.com/operate-first/toolbox/master/scripts/o1-clone?

(This path hadn't occurred to me because I didn't see our own scripts/ directory called from the Dockerfile, and wasn't sure how the parts of he repo relate to the image build.)

HumairAK · 2021-07-16T17:00:57Z

As a concept, I would prefer this script in the upstream project, yes.

Hrmm what do you mean here?

It seems to me it would be easier to just have this just added here -- instead of having another separate tool/repo to maintain & version that seemingly accomplishes something very similar.

quaid · 2021-07-16T22:55:09Z

As a concept, I would prefer this script in the upstream project, yes.

Hrmm what do you mean here?

It seems to me it would be easier to just have this just added here -- instead of having another separate tool/repo to maintain & version that seemingly accomplishes something very similar.

I think we're saying the same thing—I'm referring to /operate-first/toolbox as the upstream project. My repo was just a way to get things started.

Now that I know about it, I'd be just as happy if we extended opfcli, if that made more sense, than keeping a set of small bash scripts for this.

- I'm contributing this small script to the project, which was my intention from the start but I wasn't sure where to make the pull request against, or where to take the idea to discuss it! - As per the discussion in the PR[1] I am moving the script here. - I am unclear if this is the correct way to have that script deployed into the user's $PATH and made executable? - If the patch is accepted, I guess the answer is yes! Signed-off by: Karsten Wade <[email protected]> <[email protected]>

quaid · 2021-07-24T01:02:16Z

As a concept, I would prefer this script in the upstream project, yes.

Hrmm what do you mean here?

It seems to me it would be easier to just have this just added here -- instead of having another separate tool/repo to maintain & version that seemingly accomplishes something very similar.

OK @HumairAK let me know if I have this correct, thanks!

HumairAK · 2021-07-28T13:35:23Z

Dockerfile

+    curl -o /usr/local/bin/o1-clone https://github.com/operate-first/toolbox/blob/master/scripts/o1-clone && \
+    chmod +x /usr/local/bin/o1-clone

 COPY scripts/* /usr/local/bin/


@quaid by adding it to the scripts folder, this line will auto consume your script, so you don't need to add any changes to the Dockerfile for this PR. So you can remove the curl/chmod commands you have added.

Ha! Yes, indeed, it's right there. I'll blame my lack of familiarity with Dockerfiles for missing the obvious., :)

If the new script is in the `scripts` folder, it is already being copied into `/usr/local/bin`.

HumairAK · 2021-08-04T01:16:37Z

Dockerfile

    version.helm_secrets="${HELM_SECRETS_VERSION}" \
    version.ksops="${KSOPS_VERSION}" \
-    version.sops="${SOPS_VERSION}"
+    version.sops="${SOPS_VERSION}" \


can you remove this new slash \

Finger slip I'm sure :) fixed

quaid · 2021-08-06T17:02:56Z

@quaid: The following test failed, say /retest to rerun all failed tests:
Test name Commit Details Rerun command
aicoe-ci/prow/pre-commit 21d17af link /test pre-commit

Full PR test history. Your PR dashboard. Please help us and open an issue when you hit one in your PR.

This seems to be a problem with the testing, where shall I open an issue?

The problem is, I used the text editor in the browser to remove the / character then committed. So of course there was no pre-commit run because I'm not on my local system.

The only way I see around this is forcing the merge, which doesn't seem like the behavior we want.

Can @sesheta test for when the commit happens via the browser and skip the unnecessary tests?

HumairAK · 2021-08-09T20:14:35Z

Dockerfile

    curl -o /usr/local/bin/yq -L https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64 && \
    chmod +x /usr/local/bin/yq
-
+    


last change -- can you remove the whitespace here, as this is failing the pre-commit check.

OK, I think I got this, and I appreciate the connection—pre-commit legitimately failed, now I know to look further when that happens here.
/retest

ok, now it should clear pre-commit check

HumairAK · 2021-08-17T19:30:26Z

/lgtm
/approve

sesheta · 2021-08-17T19:30:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HumairAK

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [HumairAK]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

quaid requested a review from tumido July 13, 2021 22:51

quaid requested review from 4n4nd and HumairAK as code owners July 13, 2021 22:51

sesheta requested a review from larsks July 13, 2021 22:51

sesheta added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jul 13, 2021

Missed gluing the executable lines together

0ba09e0

- I'm new at this Dockerfile thing, and I realized when viewing the PR online what I had missed. - Live and learn :) Signed-off by: Karsten Wade <[email protected]> <[email protected]>

quaid changed the title ~~Adding mini CLI tool for cloning repos~~ Adding mini CLI script for cloning repos Jul 16, 2021

sesheta added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 24, 2021

HumairAK reviewed Jul 28, 2021

View reviewed changes

Removing double-installation

871b268

If the new script is in the `scripts` folder, it is already being copied into `/usr/local/bin`.

quaid requested a review from HumairAK August 3, 2021 23:00

HumairAK reviewed Aug 4, 2021

View reviewed changes

removing stray character

21d17af

quaid requested a review from HumairAK August 6, 2021 16:46

HumairAK reviewed Aug 9, 2021

View reviewed changes

Removing whitespace

3a00dd2

ok, now it should clear pre-commit check

sesheta assigned HumairAK Aug 17, 2021

sesheta added the lgtm Indicates that a PR is ready to be merged. label Aug 17, 2021

sesheta added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 17, 2021

sesheta merged commit a288437 into master Aug 17, 2021

harshad16 deleted the quaid-clone-tool-idea branch April 29, 2022 08:33

harshad16 restored the quaid-clone-tool-idea branch April 29, 2022 08:33

		curl -o /usr/local/bin/yq -L https://github.com/mikefarah/yq/releases/download/${YQ_VERSION}/yq_linux_amd64 && \
		chmod +x /usr/local/bin/yq

Conversation

quaid commented Jul 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issues and Dependencies

This introduces a breaking change

This Pull Request implements

Description

Uh oh!

tumido commented Jul 15, 2021

Uh oh!

quaid commented Jul 15, 2021

Uh oh!

quaid commented Jul 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HumairAK commented Jul 16, 2021

Uh oh!

quaid commented Jul 16, 2021

Uh oh!

quaid commented Jul 24, 2021

Uh oh!

HumairAK Jul 28, 2021

Choose a reason for hiding this comment

Uh oh!

quaid Aug 3, 2021

Choose a reason for hiding this comment

Uh oh!

HumairAK Aug 4, 2021

Choose a reason for hiding this comment

Uh oh!

quaid Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

quaid commented Aug 6, 2021

Uh oh!

HumairAK Aug 9, 2021

Choose a reason for hiding this comment

Uh oh!

quaid Aug 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HumairAK commented Aug 17, 2021

Uh oh!

sesheta commented Aug 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

quaid commented Jul 13, 2021 •

edited

Loading

quaid commented Jul 16, 2021 •

edited

Loading

quaid Aug 17, 2021 •

edited

Loading