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
new workspace tracking #29
Conversation
src/Runner.Worker/TrackingConfig.cs
Outdated
Repositories[repoFullName] = new RepositoryTrackingConfig() | ||
{ | ||
LastRunOn = DateTimeOffset.Now, | ||
RepositoryPath = Path.Combine(PipelineDirectory, repoName) |
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.
NIt: This can just be RepositoryPath = WorkspaceDirectory
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.
fixed
#else | ||
return await ExecuteDockerCommandAsync(context, "login", $"--username \"{username}\" --password-stdin {server}", new List<string>() { password }, context.CancellationToken); | ||
#endif | ||
return await ExecuteDockerCommandAsync(context, "login", $"--username \"{username}\" --password \"{password.Replace("\"", "\\\"")}\" {server}", context.CancellationToken); |
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.
I'm not quite sure what's happening here - why are we able to unify this? Or maybe the better question is what problem were we originally trying to solve by splitting it out into different platforms/using stdin? I think I'm just missing context
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.
we don't need this anymore. :D
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.
I was waiting on other PR to be merged first.
if (selfRepo.Endpoint != null) | ||
{ | ||
var repoEndpoint = message.Resources.Endpoints.FirstOrDefault(x => x.Id == selfRepo.Endpoint.Id); | ||
if (repoEndpoint?.Authorization?.Parameters != null && repoEndpoint.Authorization.Parameters.ContainsKey("accessToken")) |
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.
Nit: can we just do a TryGetValue
to get githubAccessToken
here?
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.
this is for compat, will goes away after Jason's server change goes in.
650ccde
to
47a9f21
Compare
47a9f21
to
b484e2c
Compare
github_token vs GITHUB_TOKEN Resolves actions#29
No description provided.