Interactive cli #83

juampynr · 2022-07-01T22:46:54Z

This is raw and lacks error handling and best practices but already does something:

Upon compiling the Go application, you get a drupal9ci executable.
Copying this executable into a Drupal project and running it shows the following prompt:

/Users/jrequena/projects/drupal [master] ./drupal9ci 
Use the arrow keys to navigate: ↓ ↑ → ← 
? Select CI provider: 
  ▸ Bitbucket
    CircleCI
    GitHub Actions
    GitLab CI
    Travis CI

Selecting an option above performs the curl request to the installer command and executes it.

As I am writing this I think that I can simply run the script without having to download it since it is packaged within the executable. I will fix this in this pull request.

Once we have this in place we could:

Set up a job that creates releases automatically out of tags. Users would simply download the executable and run it.
Start replacing the bash scripts by actual Go code that we can test easily.

Lists available CI providers and executes the respective script

juampynr · 2022-07-01T22:47:52Z

cmd/root.go

+import (
+	"fmt"
+	"github.com/manifoldco/promptui"
+	"github.com/spf13/cobra"


cobra is a tool to create CLI interfaces in Go.

juampynr · 2022-07-01T22:48:33Z

cmd/root.go

+	"fmt"
+	"github.com/manifoldco/promptui"
+	"github.com/spf13/cobra"
+	"github.com/spf13/viper"


viper is a tool to manage configuration. It pairs well with cobra. I have not used it yet but decided to get it in already since I am sure that we will make use of it.

juampynr · 2022-07-04T22:37:33Z

cmd/root.go

+	var setupScript string
+	var err error
+
+	if len(os.Args) > 1 {


Had to add this in order to set a CI provider at the CircleCI job that tests the application.

juampynr · 2022-07-04T22:38:50Z

@fjgarlin thoughts about this so far?

fjgarlin · 2022-07-05T08:23:50Z

Code-wise, it all looks good (my knowledge of go is limited but it's all very readable).

I'd also expect as part of this PR changes in the readme file explaining how to use the new tool.
Looking at the code, do we need to compile the command with go first locally, and then run it?

--
starts reading the very first comment...

fjgarlin · 2022-07-05T08:27:52Z

You just addressed it
Users would simply download the executable and run it. that would address it.

I'll test it all locally, just wanted to give some quick feedback as I found it.

juampynr · 2022-07-05T08:50:38Z

Code-wise, it all looks good (my knowledge of go is limited but it's all very readable).

I'd also expect as part of this PR changes in the readme file explaining how to use the new tool.

Looking at the code, do we need to compile the command with go first locally, and then run it?

-- starts reading the very first comment...

1: yep! I just adjusted that
2: no, I added a GitHub action that should create a release with the resulted binary that should be ready to use.

fjgarlin · 2022-07-05T09:13:07Z

This is so awesome, it opens the door to future improvements like rewriting bash scripts in go, choosing the PHP version (once we figure out images), etc. And most importantly, it lowers the entry barrier to this project.

A super small suggestion. When you run the command, after choosing the platform, it takes around 5~10 seconds until you get the first line of output. For a second I wasn't sure if my machine was hanging, so it might be good to put a Starting... line so the user knows they'll need to wait a few seconds.

So something like:

? Select CI provider: 
  ▸ Bitbucket
    CircleCI
    GitHub Actions
    GitLab CI
    Travis CI
    
 ✔ Bitbucket
 Starting... 
 (5~10 seconds later)
 output: + wget -O...

Could be Starting... or This might take a few seconds... or something like that.

fjgarlin · 2022-07-05T09:15:25Z

cmd/root.go

+	if len(args) > 1 {
+		return &args[1], nil
+	}
+	ciProviders := []string{"Bitbucket", "CircleCI", "GitHub Actions", "GitLab CI", "Travis CI"}


I'm new to go language. Can we not import/use the const defined in scripts/scripts.go to avoid repetition? If we can't that's totally fine.

fjgarlin · 2022-07-05T09:18:41Z

You missed replacing the old command with the new one here: https://github.com/Lullabot/drupal9ci/blob/82-interactive-cli/README.md#github-actions

juampynr · 2022-07-05T22:07:41Z

@fjgarlin this is ready for another look.

codecov-commenter · 2022-07-06T20:50:39Z

Codecov Report

❗ No coverage uploaded for pull request base (master@519a879). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #83   +/-   ##
=========================================
  Coverage          ?   40.54%           
=========================================
  Files             ?        1           
  Lines             ?       37           
  Branches          ?        0           
=========================================
  Hits              ?       15           
  Misses            ?       21           
  Partials          ?        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 519a879...6309765. Read the comment docs.

juampynr · 2022-07-06T22:07:27Z

@fjgarlin I will merge this tomorrow. Let me know if you find any issues. We can work on further changes in new pull requests.

fjgarlin · 2022-07-07T08:41:06Z

I was away from the computer yesterday and couldn't check the activity. I've been checking the code changes and everything looks really solid.

I think it's good to be merged. Approving the PR. Great work!

fjgarlin · 2022-07-07T08:45:13Z

.github/workflows/publish-docker-image.yml

+
+jobs:
+  publish-docker-image:
+    name: Publish Docker image


Additional questions, but PR is good to be merged:

Does this publish the image generated from Dockerfile into github? or is it another unrelated thing?

If it's the image from Dockerfile, how would we use it later in the different platforms?

Additional questions, but PR is good to be merged:

Does this publish the image generated from Dockerfile into github? or is it another unrelated thing?

Yes, here: https://github.com/Lullabot/drupal9ci/pkgs/container/drupal9ci

If it's the image from Dockerfile, how would we use it later in the different platforms?

I suggest that we start supporting only the Dockerile9 (I replaced Dockerfile with Dockerfile9). Next, we can improve the cli so it asks for Drupal version and picks the right Dockerfile (at which point we could restore the old Dockerfile and build and publish both images in this job). Alternatively, I wonder if we can detect the version and pick the Dockerfile automatically. Thoughts?

I think giving the user the option to choose might be enough. We can just give them options like this:

* Dockerfile9: php8.1, composer 2, npm 16... * Dockerfile: php7.4, composer 1, npm 12... * Choose your own image: this might not work out of the box. You'll be prompted to type it.

Then with that info, we can search/replace the config file of the platform they've used.

Yep! I like that.

fjgarlin · 2022-07-07T08:45:40Z

.github/workflows/test.yml

@@ -0,0 +1,41 @@
+name: Test


Loving the additional tests

Yep! This is just the beginning. I want to move the shell scripts into easily testable Go code as you suggested.

Notice that I kind of hacked the default Cobra command: I removed sub-commands so you can just run drupal9ci but you can certainly have multiple sub commands so you could do, for example drupal9ci check to check for requirements and, when ready, run drupal9ci install to run the installer. We could also do this in a follow up.

At some point (sorry I am throwing out a lot of thoughts here), I would like to rename (again, I know) this repository to drupalci since we will be able to support multiple Drupal versions. Thoughts about this?

100% on this.

juampynr · 2022-07-07T09:02:55Z

@fjgarlin I may have broken some of the providers by updating the Docker images. I am going to merge and then will start testing them one by one with a sample repository. Feel free to suggest whatever fixes you may consider.

fjgarlin · 2022-07-07T09:54:38Z

Not a problem. As this is not a package that is updated on people's projects, but rather a "use once, then tweak in your own copy", we will not be breaking people's projects.

If new users find issues I'm sure they'll report them. But yeah, in any case, it's a good idea to give it a quick try by yourself and if I find time next week I can also do the same, but all of it as follow-ups.

juampynr added 2 commits July 2, 2022 00:35

[#82] Initial command line interface

fa5de26

Lists available CI providers and executes the respective script

[#82] Ignore the resuting command from running go build

3e9f9c3

juampynr commented Jul 1, 2022

View reviewed changes

juampynr added 5 commits July 5, 2022 00:19

[#82] Embed scripts in binary

a633d18

[#82] No Juampy, this is not GitHub Actions XD

2388005

[#82] Install go

2f60608

[#82] Install go and build app in single step

70dd150

[#82] Fix CI provider at CI job

151ab46

juampynr commented Jul 4, 2022

View reviewed changes

[#82] Create release binaries workflow

d980d56

juampynr marked this pull request as ready for review July 5, 2022 08:15

juampynr requested a review from fjgarlin July 5, 2022 08:15

[#82] Update docs

135dd36

juampynr added 2 commits July 5, 2022 10:34

[#82] Adjust README notes

28ab0d2

[#82] Refactor command

4a919fc

juampynr added 3 commits July 5, 2022 11:02

[#82] Test creating release and publishing Docker image

67a514d

[#82] Add branch to test workflow

4988aef

[#82] Adjust workflow conditions

199e869

[#82] Move build and test job to GitHub Actions

c8c951b

fjgarlin reviewed Jul 5, 2022

View reviewed changes

[#82] Set tags to latest

d823cc9

juampynr added 2 commits July 5, 2022 11:26

[#82] Set release tag to sha

543aba5

[#82] Remove curl command from README

2e173ee

juampynr added 5 commits July 5, 2022 23:48

[#82] Set tag and commit in release action

802a63a

[#82] Yet another release action plugin

cc9c248

[#82] Fix syntax

2f4385b

[#82] Use ref as tag name

be537a9

[#82] Fix syntax to select branches

f6a192c

juampynr requested a review from fjgarlin July 5, 2022 22:07

juampynr added 10 commits July 6, 2022 10:08

[#82] Only publish Docker image on merge to master

81adb7b

[#82] Refactor CI provider list

5768da4

[#82] Add noop CircleCI job

c28f8eb

[#82] Use sha for release

6acb2a1

[#82] Set docker image at CircleCI noop job

3472d5c

[#82] Set short sha for release

fde0259

[#82] Fix job name for CircleCI

0b7490b

[#82] Add code coverage job

6447573

[#82] Add first test

60d197c

[#82] Add test to LoadSetupScripts

49e02eb

juampynr added 2 commits July 6, 2022 23:58

[#82] Add badges

7808d7c

[#82] Update codecov action

cd41095

fjgarlin approved these changes Jul 7, 2022

View reviewed changes

fjgarlin reviewed Jul 7, 2022

View reviewed changes

[#82] Replace Drupal8 Dockerfile with Drupal 9 one

6309765

juampynr merged commit db09477 into master Jul 7, 2022

juampynr mentioned this pull request Jul 7, 2022

Interactive command line interface #82

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Interactive cli #83

Interactive cli #83

juampynr commented Jul 1, 2022

juampynr Jul 1, 2022

juampynr Jul 1, 2022

juampynr Jul 4, 2022

juampynr commented Jul 4, 2022

fjgarlin commented Jul 5, 2022 •

edited

fjgarlin commented Jul 5, 2022

juampynr commented Jul 5, 2022

fjgarlin commented Jul 5, 2022

fjgarlin Jul 5, 2022

juampynr Jul 5, 2022

fjgarlin commented Jul 5, 2022

juampynr commented Jul 5, 2022

codecov-commenter commented Jul 6, 2022 •

edited by codecov bot

juampynr commented Jul 6, 2022

fjgarlin commented Jul 7, 2022

fjgarlin Jul 7, 2022

juampynr Jul 7, 2022 •

edited

fjgarlin Jul 7, 2022

juampynr Jul 7, 2022

fjgarlin Jul 7, 2022

juampynr Jul 7, 2022

juampynr Jul 7, 2022

fjgarlin Jul 7, 2022

juampynr commented Jul 7, 2022

fjgarlin commented Jul 7, 2022

Interactive cli #83

Interactive cli #83

Conversation

juampynr commented Jul 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juampynr commented Jul 4, 2022

fjgarlin commented Jul 5, 2022 • edited

fjgarlin commented Jul 5, 2022

juampynr commented Jul 5, 2022

fjgarlin commented Jul 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjgarlin commented Jul 5, 2022

juampynr commented Jul 5, 2022

codecov-commenter commented Jul 6, 2022 • edited by codecov bot

Codecov Report

juampynr commented Jul 6, 2022

fjgarlin commented Jul 7, 2022

Choose a reason for hiding this comment

juampynr Jul 7, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juampynr commented Jul 7, 2022

fjgarlin commented Jul 7, 2022

fjgarlin commented Jul 5, 2022 •

edited

codecov-commenter commented Jul 6, 2022 •

edited by codecov bot

juampynr Jul 7, 2022 •

edited