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

chore(infra-jobs): remove githubCheckNames #5025

Conversation

lemeurherve
Copy link
Member

@lemeurherve lemeurherve commented Feb 26, 2024

@lemeurherve lemeurherve requested review from dduportal, smerle33 and a team February 26, 2024 16:45
@lemeurherve lemeurherve enabled auto-merge (squash) February 26, 2024 16:55
@lemeurherve lemeurherve added chore Chore/Non Functional Task/Cleanup infra.ci.jenkins.io labels Feb 26, 2024
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

FWIW those status checks names are incredible long =/

@lemeurherve lemeurherve merged commit 3eb3fac into jenkins-infra:main Feb 27, 2024
9 checks passed
@lemeurherve lemeurherve deleted the helpdesk2778-remove-updatecli-job-description branch February 27, 2024 12:05
@lemeurherve
Copy link
Member Author

lemeurherve commented Feb 27, 2024

FWIW those status checks names are incredible long =/

I'd suggest removing:

  • "jenkins" prefix:
    • Does not add meaningful information
    • Implied by the instance name coming just after
  • The job name:
    • There isn't any case where two jobs are running in the same folder AFAIK
    • It's duplicated between jobs running on the same instance
    • It can easily be determined from:
      • the current repo
      • hover on link
  • And maybe the "pr-merge/branch" suffix:
    • Always "pr-merge" on PR
    • Always "branch" otherwise
      (AFAIK)

From:

jenkins/infra.ci.jenkins.io/kubernetes-jobs/kubernetes-management/pr-merge
jenkins/infra.ci.jenkins.io/updatecli/kubernetes-management/pr-merge

To:

infra.ci.jenkins.io/kubernetes-jobs/pr-merge
infra.ci.jenkins.io/updatecli/pr-merge

Or even:

infra.ci.jenkins.io/kubernetes-jobs
infra.ci.jenkins.io/updatecli

Answering the two important questions about status checks:

  • Where are they running
  • What are their kind

WDYT @timja @dduportal @smerle33?

@timja
Copy link
Member

timja commented Feb 27, 2024

I would think about what is important.

to me that's the check name and it should be first.
There's no need to use / to make it harder to read.

to me its:
kubernetes-management (infra.ci) or even jenkins or build for that folder.

do you even need the jenkins instance for updatecli?
updatecli or updatecli (infra.ci)

it'll be in the actor's name of who creates the check anyway

@lemeurherve
Copy link
Member Author

lemeurherve commented Feb 27, 2024

Agree, the thing is there are some constraints to get it automatically named AFAIU cf @dduportal's explanations at jenkins-infra/helm-charts#1081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Chore/Non Functional Task/Cleanup infra.ci.jenkins.io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants