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

Cluster connection bump #20885

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

audrastump
Copy link
Member

Task name:
AzureFunctionOnKubernetesV1
Description:
Calling new fleet kubeconfig pipeline function
Documentation changes required: (Y/N)
N
Added unit tests: (Y/N)
N
Attached related issue: (Y/N)
N
Checklist:

  • Task version was bumped - please check instruction how to do it
  • Checked that applied changes work as expected

return this.loadClusterType(connectionType).getKubeConfigForFleet(azureSubscriptionEndpoint, resourceGroup, clusterName, useClusterAdmin)
}else{
return this.loadClusterType(connectionType).getKubeConfig(azureSubscriptionEndpoint, resourceGroup, clusterName, useClusterAdmin)
}
} else {
return this.loadClusterType(connectionType).getKubeConfig()
}
Copy link

@nwnt nwnt Feb 26, 2025

Choose a reason for hiding this comment

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

I know this is not your change, but I have some major OCD with else clauses in general. We can just avoid this by returning early. i.e. something like this:

if (connectionType !== "azureResourceManager") {
  return this.loadClusterType(connectionType).getKubeConfig()
}

// continue with everything else in the if clause

Copy link

Choose a reason for hiding this comment

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

See more here about why we should return early. TLDR, it simplifies things by having minimal nest control.

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