Skip to content

Conversation

@danudey
Copy link
Contributor

@danudey danudey commented Jul 4, 2025

A simple patch to fix a pet peeve of mine. Does the following:

  • Refactor ContextList() -> ContextNameList(); now more accurately describes what it does in the context (ha!) of the next change
  • Add ContextList() to return list of context objects rather than just names
  • Add completion for sem context by creating a ValidArgsFunction
  • Add a warning if the user is trying to complete sem context but we don't have any contexts configured

Note: We bump cobra in this PR, but I looked through the changelogs between this version (1.9.1) and the previous (1.7.0) and there are no backwards-incompatible changes anywhere that I saw. Bump was for two reasons:

  1. It's always good to have up-to-date dependencies
  2. It added some convenience functions and types that make the code a lot cleaner and more readable.

@hamir-suspect hamir-suspect requested a review from lucaspin July 9, 2025 09:16
@lucaspin
Copy link
Contributor

lucaspin commented Jul 9, 2025

/sem-approve

@lucaspin
Copy link
Contributor

lucaspin commented Jul 9, 2025

@danudey thanks for the PR, everything looks good. However, you're gonna need to update your branch with master, to pick up this change. With that change, your CI build should pass.

danudey added 4 commits July 9, 2025 10:37
1. Add `contextValidArgs()` function to complete a list of contexts and
   their endpoints
2. Set the `context` subcommand's `ValidArgsFunction` to
   `contextValidArgs()`
@danudey danudey force-pushed the add-sem-context-completion branch from d49fd4c to ef0bec0 Compare July 9, 2025 17:38
@danudey
Copy link
Contributor Author

danudey commented Jul 9, 2025

@danudey thanks for the PR, everything looks good. However, you're gonna need to update your branch with master, to pick up this change. With that change, your CI build should pass.

@lucaspin Rebased and pushed, it's all approved now but it looks like CI didn't run? Or re-run?

@lucaspin
Copy link
Contributor

lucaspin commented Jul 9, 2025

/sem-approve

@lucaspin lucaspin merged commit a7e46d0 into semaphoreci:master Jul 9, 2025
1 check passed
@lucaspin
Copy link
Contributor

lucaspin commented Jul 9, 2025

All good now. Thanks again @danudey!

@danudey danudey deleted the add-sem-context-completion branch July 10, 2025 20:34
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.

2 participants