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(): Bugfix and refactor #279

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

chore(): Bugfix and refactor #279

wants to merge 6 commits into from

Conversation

bitmaskit
Copy link
Contributor

@bitmaskit bitmaskit commented Mar 5, 2025

Description

  • fix getDownloadUrlByTag
  • cache piper binary (better caching capability will be provided with actions/cache when we start to use single binary)
  • organized build functions into build.ts

@bitmaskit bitmaskit force-pushed the bugfix-and-refactor branch from ddf9b2c to b2ffae1 Compare March 5, 2025 11:02
@@ -124,5 +124,5 @@ export function getDownloadUrlByTag (version: string, forAPICall: boolean = fals
version = version.toLowerCase()
return (version === '' || version === 'master' || version === 'latest')
? `${GITHUB_COM_SERVER_URL}/SAP/jenkins-library/releases/latest`
: `${GITHUB_COM_SERVER_URL}/SAP/jenkins-library/releases/${forAPICall ? 'tags' : 'tag'}/${version}`
: `${forAPICall ? `${GITHUB_COM_API_URL}/repos` : GITHUB_COM_SERVER_URL}/SAP/jenkins-library/releases/${forAPICall ? 'tags' : 'tag'}/${version}`
Copy link
Member

@Googlom Googlom Mar 5, 2025

Choose a reason for hiding this comment

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

this will cause errors because all API requests must be authorized and but most of the pipelines don't have github.com token configured.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a helper function, if someone needs a download Url for API call they need to call this function with forAPICall: true. We dont have such call for now. It's responsibility of the caller (in the future) to make sure token is provided.

@bitmaskit bitmaskit marked this pull request as ready for review March 5, 2025 13:48
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