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

Automatic generation of README catalogs #737

Closed
wants to merge 1 commit into from

Conversation

YuviGold
Copy link
Contributor

Description

Resolves #736

The README.md tables of the catalog of CLIs and apps might
become out of sync from time to time.
It should be automatically generated and fail build in case not committed.

Using cog https://nedbatchelder.com/code/cog/ , the README.md file
can be generated and overwrite itself by calling

  • arkade install --print-table
  • arkade get --output markdown

The Github Action workflows were updated accordingly to run the
make lint as part of the job, in order to verify calling make docs
does not make any changes to the repoisotry that weren't commited to
git.

Add ${PWD}/${BIN} to the Makefile in order to use the latest arkade
that was generated by make dist.

Bugfix arkade install --print-table to be key ordered (otherwise
everytime it produces a different table).

Motivation and Context

  • I have raised an issue to propose this change, which has been given a label of design/approved by a maintainer (required)

How Has This Been Tested?

  • Changing a tool and expecting the job to fail.
➜  arkade git:(cog-generate-readme) sed -i 's/terraform/terra/g' ./pkg/get/tools.go
  • Rebuilding
➜  arkade git:(cog-generate-readme) ✗ make dist
mkdir -p bin
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -a -installsuffix cgo -o bin/arkade
CGO_ENABLED=0 GOOS=darwin go build -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -a -installsuffix cgo -o bin/arkade-darwin
CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build -a -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -installsuffix cgo -o bin/arkade-darwin-arm64
CGO_ENABLED=0 GOOS=linux GOARCH=arm GOARM=6 go build -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -a -installsuffix cgo -o bin/arkade-armhf
CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -a -installsuffix cgo -o bin/arkade-arm64
CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build -ldflags "-s -w -X github.com/alexellis/arkade/cmd.Version=0.8.33-1-g994e784-dirty -X github.com/alexellis/arkade/cmd.GitCommit=994e7843f373e366bfafb795c46e11ffa63904d9" -a -installsuffix cgo -o bin/arkade.exe
  • Running the generation + linter
➜  arkade git:(cog-generate-readme) ✗ make lint
cog -r README.md
Cogging README.md  (changed)
git diff --exit-code  # fail if generation caused any diff
diff --git a/README.md b/README.md
index febdd97..7fb23af 100644
--- a/README.md
+++ b/README.md
@@ -650,7 +650,7 @@ cog.outl(subprocess.getoutput("arkade get --output markdown"))
 | sops             | Simple and flexible tool for managing secrets                                                                                             |
 | stern            | Multi pod and container log tailing for Kubernetes.                                                                                       |
 | talosctl         | The command-line tool for managing Talos Linux OS.                                                                                        |
-| terraform        | Infrastructure as Code for major cloud providers.                                                                                         |
+| terra            | Infrastructure as Code for major cloud providers.                                                                                         |
 | terragrunt       | Terragrunt is a thin wrapper for Terraform that provides extra tools for working with multiple Terraform modules                          |
 | terrascan        | Detect compliance and security violations across Infrastructure as Code.                                                                  |
 | tfsec            | Security scanner for your Terraform code                                                                                                  |
diff --git a/pkg/get/tools.go b/pkg/get/tools.go
index 3b99842..0a8d58e 100644
--- a/pkg/get/tools.go
+++ b/pkg/get/tools.go
@@ -657,8 +657,8 @@ https://github.com/inlets/inletsctl/releases/download/{{.Version}}/{{$fileName}}
 	tools = append(tools,
 		Tool{
 			Owner:       "hashicorp",
-			Repo:        "terraform",
-			Name:        "terraform",
+			Repo:        "terra",
+			Name:        "terra",
 			Version:     "1.1.9",
 			Description: "Infrastructure as Code for major cloud providers.",
 			URLTemplate: `
make: *** [Makefile:45: lint] Error 1

As you can see, the README.md file was updated, but wasn't committed to git.

In addition, locally using https://github.com/nektos/act to call the repository Github Actions.

Are you a GitHub Sponsor yet (Yes/No?)

  • Yes
  • No

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Documentation

  • I have updated the list of tools in README.md if (required) with ./arkade get -o markdown
  • I have updated the list of apps in README.md if (required) with ./arkade install --help

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have tested this on arm, or have added code to prevent deployment

The `README.md` tables of the catalog of CLIs and apps might
become out of sync from time to time.
It should be automatically generated and fail build in case not committed.

Using `cog` https://nedbatchelder.com/code/cog/ , the `README.md` file
can be generated and overwrite itself by calling

- `arkade install --print-table`
- `arkade get --output markdown`

The Github Action workflows were updated accordingly to run the
`make lint` as part of the job, in order to verify calling `make docs`
does not make any changes to the repoisotry that weren't commited to
git.

Add `${PWD}/${BIN}` to the Makefile in order to use the latest `arkade`
that was generated by `make dist`.

Bugfix `arkade install --print-table` to be key ordered (otherwise
everytime it produces a different table).

Signed-off-by: Yuval Goldberg <yuvigoldi@gmail.com>
@Shikachuu
Copy link
Contributor

Shikachuu commented Aug 15, 2022

First of all I like the general idea of auto-updating the readme, for that we eventually need to build better tooling for sure!

But tbh, we could solve something like this with basic unix tooling like sed and regular shell scripts and personally I'm not really a fan of using a python to achieve the same result.

I assume that if we want to run this locally, we need to install python and install this python package globally with pip, like you did in the CI update, right?

But that's only my two cents. What do you think @alexellis ?

@YuviGold
Copy link
Contributor Author

YuviGold commented Aug 15, 2022

First of all I like the general idea of auto-updating the readme, for that we eventually need to build better tooling for sure!

But tbh, we could solve something like this with basic unix tooling like sed and regular shell scripts and personally I'm not really a fan of using a python to achieve the same result.

I assume that if we want to run this locally, we need to install python and install this python package globally with pip, like you did in the CI update, right?

But that's only my two cents. What do you think @alexellis ?

Thanks @Shikachuu . I understand that it requires more dependencies. Honestly cog is very powerful for static files generation and you can do any manipulation you'd like with Python this way. The inplace code makes it very user friendly.
The Makefile's docs implementation can be changed to any other tool, just like sed. But IMO it will be harder to read and maintain.

Another suggestion, if you want, I can make it encapsulated inside a docker container, so you won't be required to have Python nor cog but only docker available. (something like https://3musketeers.io/ methodology)

Please let me know what do you prefer. :)

@Shikachuu
Copy link
Contributor

Shikachuu commented Aug 15, 2022

Hey @YuviGold,

Thanks for the detailed reply!

I understand the concern from your side about sed and shell scripts, but the probability that a go developer is familiar with shell scripts is higher then python's, plus we already have a heaps of scripts in the hack folder already, so my opinion stands.

If you insist, I think the later is better, using cog as an encapsulated container.

But let's see what the others think about this.

- name: "Set up Python"
uses: "actions/setup-python@v4"
with:
python-version: '3.10'
Copy link
Owner

Choose a reason for hiding this comment

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

In my opinion, a CI job is not the correct place to regenerate a README file, which needs to be committed back into the tree.

This should be a task that contributors run with PRs, and part of the PR template.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion, a CI job is not the correct place to regenerate a README file, which needs to be committed back into the tree.

This should be a task that contributors run with PRs, and part of the PR template.

+1 on this, that's why I asked about how to run this locally!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea behind it was that the CI would rerun the generation script (make lint that calls make docs) and verify no changes weren't committed to git.
Of course it can be removed from the publish GHA

@alexellis
Copy link
Owner

But tbh, we could solve something like this with basic unix tooling like sed and regular shell scripts and personally I'm not really a fan of using a python to achieve the same result.

+1 to this, it can probably be achieved with a Makefile, or some Go code in the repo that can be run separately from arkade.

When solving a new problem, we want input from maintainers and the person proposing the solution, before making a firm technical decision like this.

We also need to decide whether we are bothered enough by the problem to spend time on it, because whatever we do now will need to be explained and maintained going forward also.

@YuviGold
Copy link
Contributor Author

But tbh, we could solve something like this with basic unix tooling like sed and regular shell scripts and personally I'm not really a fan of using a python to achieve the same result.

+1 to this, it can probably be achieved with a Makefile, or some Go code in the repo that can be run separately from arkade.

When solving a new problem, we want input from maintainers and the person proposing the solution, before making a firm technical decision like this.

We also need to decide whether we are bothered enough by the problem to spend time on it, because whatever we do now will need to be explained and maintained going forward also.

@alexellis @Shikachuu
No worries. I was in the mood to write that code and I think it gave more understanding over the solution concept instead of no code at all. Without it I wasn't able to think of the following bullets:

  • Generation script needs to run with the latest binary (in order to get latest changes, using bin binary)
  • Running the script as part of the CI in order to verify it wasn't missed
  • Making the script deterministic - which raised as well the bug on the apps help message

IMHO Go is not built for small scripts like that. cog is nice and powerful and might be used later in the future when the repository documentation become bigger. Its usage with docker is much friendlier and can be resolved easily.

But sure I can think of other solutions as well. Let me know how you would like to proceed. :)

@Shikachuu
Copy link
Contributor

I would prefer to continue this discuisson in the issue and close this PR since we don't have a common ground about the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automate generation of README tables
3 participants