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

fix: template of Other runs in tkn-pac describe command #2018

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

PuneetPunamiya
Copy link
Contributor

  • Before this patch the output of other runs in the tkn pac describe command was not aligned properly

  • Hence, with this patch it fixes and aligns the output

Changes

Submitter Checklist

  • 📝 Ensure your commit message is clear and informative. Refer to the How to write a git commit message guide. Include the commit message in the PR body rather than linking to an external site (e.g., Jira ticket).

  • ♽ Run make test lint before submitting a PR to avoid unnecessary CI processing. Consider installing pre-commit and running pre-commit install in the repository root for an efficient workflow.

  • ✨ We use linters to maintain clean and consistent code. Run make lint before submitting a PR. Some linters offer a --fix mode, executable with make fix-linters (ensure markdownlint and golangci-lint are installed).

  • 🧪 While 100% coverage isn't required, we encourage unit tests for code changes where possible.

  • 🔎 Address any CI test flakiness before merging, or provide a valid reason to bypass it (e.g., token rate limitations).

    (update the documentation accordingly)

Before this patch the output of other runs in the tkn pac
describe command was not aligned properly

Hence, with this patch it fixes and aligns the output

Signed-off-by: PuneetPunamiya <[email protected]>
@zakisk
Copy link
Contributor

zakisk commented Mar 24, 2025

/test

@chmouel
Copy link
Member

chmouel commented Mar 24, 2025

e2e test is not running for puneet at the moment which is expected since not yet in allowed list

@vdemeester
Copy link
Member

e2e test is not running for puneet at the moment which is expected since not yet in allowed list

But should it be successful ? because one could merge this, without the test running (vs having something failing or in pending that "would" prevent that)

@chmouel chmouel added the e2e label Mar 25, 2025
@chmouel
Copy link
Member

chmouel commented Mar 25, 2025

i solved it differently see #2024

@@ -32,7 +32,7 @@

{{ $.ColorScheme.Underline "Other Runs:" }}

{{ $.ColorScheme.Bold "STATUS:" }} {{ $.ColorScheme.Bold "Event" }} {{ $.ColorScheme.Bold "Branch" }} {{ $.ColorScheme.Bold "SHA" }} {{ $.ColorScheme.Bold "STARTED TIME" }} {{ $.ColorScheme.Bold "DURATION" }} {{ $.ColorScheme.Bold "PIPELINERUN" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

@PuneetPunamiya why colon is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's no such reason, I can add it back again

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

Successfully merging this pull request may close these issues.

4 participants