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

Add tkn approvaltask 😻 #141

Merged

Conversation

PuneetPunamiya
Copy link
Collaborator

@PuneetPunamiya PuneetPunamiya commented Apr 22, 2024

This patch bootstraps CLI and adds implementation of List Command

  • List all the approval task in the default namespace
    • If flag provided the list the approval task in that particular namespace
  • Flags
    • -A, –all-namespace → To list approvaltask from all namespaces
  • Global Flags
    • –namespace → namespace to use
  • UX
tkn approvaltask list
tkn at ls

Output

NAME   NumberOfApprovalsRequired   PendingApprovals   Rejected   STATUS
cr-1   2                           2                  0          Pending
cr-2   2                           0                  1          Rejected
cr-3   2                           0                  0          Approved

@PuneetPunamiya
Copy link
Collaborator Author

/hold

Copy link

openshift-ci bot commented Apr 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: PuneetPunamiya

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch 4 times, most recently from 3495f89 to a8ffc00 Compare April 24, 2024 04:41
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch 8 times, most recently from 162799f to ae376d6 Compare April 26, 2024 09:08
@PuneetPunamiya PuneetPunamiya changed the title [WIP] Add tkn approvaltask 😻 Add tkn approvaltask 😻 Apr 26, 2024
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch from ae376d6 to 4cfe7ef Compare May 6, 2024 11:13
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch 2 times, most recently from 5674713 to 13d2a38 Compare May 6, 2024 12:37
@PuneetPunamiya
Copy link
Collaborator Author

/hold cancel

Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch from 13d2a38 to eb7c4f3 Compare May 10, 2024 13:41
"Pending": color.FgHiBlue,
}

const listTemplate = `{{- $at := len .ApprovalTasks.Items }}{{ if eq $at 0 -}}
Copy link
Member

@piyush-garg piyush-garg May 10, 2024

Choose a reason for hiding this comment

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

This table makes not much sense for me. Too much information repeated. We can go for something like

NAME    Approvals    STATUS
foo     2/3          Wait
bar     4/4          Approved

Copy link
Member

Choose a reason for hiding this comment

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

As we are supporting only single reject, showing a column does not make any sense

Choose a reason for hiding this comment

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

@piyush-garg I am fine with it as long as its clear that in the example of 0/3 and status as "rejected" someone from the approval list has rejected it.

Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch from eb7c4f3 to b6ad294 Compare May 13, 2024 06:33
Signed-off-by: PuneetPunamiya <ppunamiy@redhat.com>
@PuneetPunamiya PuneetPunamiya force-pushed the add-tkn-approvaltask branch from b6ad294 to a7bf9fa Compare May 13, 2024 11:22
var ConditionColor = map[string]color.Attribute{
"Rejected": color.FgHiRed,
"Approved": color.FgHiGreen,
"Pending": color.FgHiBlue,
Copy link
Member

Choose a reason for hiding this comment

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

this should be yellow/orange :D

@piyush-garg
Copy link
Member

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 13, 2024
@piyush-garg piyush-garg merged commit fb45fa6 into openshift-pipelines:main May 13, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants