-
Notifications
You must be signed in to change notification settings - Fork 59
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 ability to resolve snapshot latest date #679
base: main
Are you sure you want to change the base?
Conversation
57d797e
to
cf0fd99
Compare
Update. Sorry for too many pushes that created pending jobs, I didn't know that it actually creates jobs within each push, thought needs approval for this. I've figured out how to setup locally most of the stack. As for the implementation,
Open questions without answer as for now:
UPD. Unified code with what was running for stable versions, removing |
606af3f
to
318d629
Compare
318d629
to
5777c01
Compare
@fwal , have you been able to check this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @khlopko and thank you for your PR!
This looks great, there's just some implementation details we should look at, let me know if you need any help 🙂
PS. Sorry about the slow response 😅
src/github-client.ts
Outdated
export interface GitHubClient { | ||
retryTimeout: number; | ||
hasApiToken(): boolean; | ||
getTags(page: number, limit: number): Promise<any>; | ||
} | ||
|
||
export class DefaultGitHubClient implements GitHubClient { | ||
retryTimeout: number = 5000; | ||
private githubToken: string | null; | ||
|
||
constructor(githubToken: string | null = null) { | ||
this.githubToken = githubToken || process.env.GH_TOKEN || null; | ||
} | ||
|
||
hasApiToken(): boolean { | ||
return this.githubToken != null && this.githubToken != ""; | ||
} | ||
|
||
async getTags(page: number, limit: number): Promise<any> { | ||
const url = `https://api.github.com/repos/swiftlang/swift/tags?per_page=${limit}&page=${page}`; | ||
return await this.get(url); | ||
} | ||
|
||
private async get(url: string): Promise<any> { | ||
let headers = {}; | ||
if (this.hasApiToken()) { | ||
headers = { | ||
Authorization: `Bearer ${this.githubToken}`, | ||
}; | ||
} | ||
const response = await fetch(url, { | ||
headers: headers, | ||
}); | ||
const json: any = await response.json(); | ||
return json; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider using the octokit and actions toolkit provided by GitHub to make requests?
(You can find an example of such an implementation here https://github.com/actions/toolkit/tree/main/packages/github)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will take a look, thanks for the reference!
function makeSnapshotPackage(snapshot: Snapshot, system: System): Package { | ||
if (snapshot.branch === "main") { | ||
return makePackage( | ||
"6.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fwal this one puzzles me on how to address: right now main snapshots has already moved to 6.1, and action still have hard-coded 6.0 version, and obviously this will be the issue for any next release as long as this hard-coded. And action is currently relies on this version for caching (which for main-snapshots in general questionable). From my perspective, there are two solutions: 1 - keep it manually updated, as I don't see a significant troubles for that cases; 2 - disable caching for main-branch snapshots and therefore the need to specify version. What do you think on these options, or maybe there are other ways to solve this?
5997f51
to
8c58389
Compare
Closes issue: #673
Implements resolution of development snapshots either by explicit date or by fetched tags from
swiftlang/swift
repo (similar to how Swiftly does this) by setting"main-snapshot"
or"X.Y-snapshot"
toswift-version
parameter. It relies on settingGH_TOKEN
value inenv
fromsecrect.GITHUB_TOKEN
to avoid hitting limits.Unites package information building under
getPackage
function. Includes tests for new functionality, including snapshot resolution with cached GitHub responses. Updated README with new options for versions and adding token.