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

Versions and completions #251

Merged
merged 5 commits into from
Aug 19, 2020

Conversation

displague
Copy link
Member

@displague displague commented Aug 15, 2020

Description

  • Fixes testing via Makefile make test (did not test anything before)
  • Fixes Add version subcommand #94 based on the git SHA and eventual git tags (blocked by Cut release tags from tink #74)
    • Adds tink-cli --version output
    • Because tink-server and tink-worker do not currently take parameters, the version is now emitted at start.
      • tink-server emits this at INFO level (because it is a long-lived process)
      • tink-worker emits this at DEBUG level (because the default run is quiet, afaict)
  • Adds a tink-cli completion subcommand

Why is this needed

Fixes: #94

How Has This Been Tested?

  • tink-cli completion --help
  • tink-cli completion bash
  •  cmd/tink-cli/tink-cli --version
     tink version f9500b2
    
  •  PACKET_VERSION="x" PACKET_ENV="t" ROLLBAR_TOKEN="t" cmd/tink-server/tink-server 
     {"level":"info","ts":1597460174.581983,"caller":"tink-server/main.go:30","msg":"starting version f9500b2","service":"github.com/tinkerbell/tink"}
     {"level":"info","ts":1597460174.582356,"caller":"metrics/metrics.go:58","msg":"initializing label values","service":"github.com/tinkerbell/tink"}
    
  •  TINKERBELL_CERT_URL="l" PACKET_VERSION="x" PACKET_ENV="t" ROLLBAR_TOKEN="t" WORKER_LOG_LEVEL=debug cmd/tink-worker/tink-worker
     DEBU[0000] Starting version f9500b2                     
     INFO[0000] RETRY_INTERVAL not set. Using default, 3 seconds 
     INFO[0000] MAX_RETRY not set. Using default, 3 retries. 
     FATA[0000] fetch cert: Get "l": unsupported protocol scheme ""
    

How are existing users impacted? What migration steps/scripts do we need?

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • added unit or e2e tests
  • provided instructions on how to upgrade

Signed-off-by: Marques Johansson <marques@packet.com>
Signed-off-by: Marques Johansson <marques@packet.com>
Signed-off-by: Marques Johansson <marques@packet.com>
Signed-off-by: Marques Johansson <marques@packet.com>
Copy link
Contributor

@gianarb gianarb left a comment

Choose a reason for hiding this comment

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

Great, but I would like to get the documentation updated about how to setup completion as part of this work.

@DailyAlice can help to find a good place for that I think.

PR is great! Thank you for taking care of it

@displague
Copy link
Member Author

displague commented Aug 17, 2020

@gianarb I can move completions to another PR and rebase either this PR or that one after merge.

If it helps, I can create a markdown and/or man-page generation process here. packet-cli performs a similar step using spf13/cobra. I can add this as a separate PR.

https://github.com/packethost/packet-cli/tree/master/docs is generated on each make invocation and commits must include docs/ without build jitter. What are your thoughts, @DailyAlice?

@gianarb
Copy link
Contributor

gianarb commented Aug 17, 2020

I don't understand @displague . I think we should just write somewhere how to set up completion
https://kubernetes.io/docs/tasks/tools/install-kubectl/#enabling-shell-autocompletion

Copy link
Contributor

@gianarb gianarb left a comment

Choose a reason for hiding this comment

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

It looks like documentation is on hold for now. This is good to go!

@gianarb gianarb added the ready-to-merge Signal to Mergify to merge the PR. label Aug 19, 2020
@mergify mergify bot merged commit ace7f59 into tinkerbell:master Aug 19, 2020
@displague displague deleted the versions-and-completions branch August 19, 2020 17:35
@mmlb mmlb removed the ready-to-merge Signal to Mergify to merge the PR. label Jan 25, 2021
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.

Add version subcommand
3 participants