add variable commit_date
#308
Conversation
|
@crazy-max any code review? |
src/meta.ts
Outdated
| this.version = this.getVersion(); | ||
| } | ||
|
|
||
| private getCommitDate(): Date { | ||
| const myOutput = execSync('git show -s --format="%ci" HEAD'); |
There was a problem hiding this comment.
We should use our toolkit for this. Can you open a PR to add this in https://github.com/docker/actions-toolkit/blob/main/src/git.ts?
There was a problem hiding this comment.
Also we should only invoke git command if context is git like we do in
metadata-action/src/context.ts
Lines 39 to 43 in 35e9aff
If context is workflow then we should take it from GH API if feasible. commits.timestamp looks to be a good candidate:
There was a problem hiding this comment.
We should use our toolkit for this. Can you open a PR to add this in https://github.com/docker/actions-toolkit/blob/main/src/git.ts?
add a new property to GitContext looks complex...
There was a problem hiding this comment.
add a new property to
GitContextlooks complex...
Just smth similar to https://github.com/docker/actions-toolkit/blob/f1593e3aa24d872bd4fa0a8f76a3bdaf26d20872/src/git.ts#L101-L103 is enough.
There was a problem hiding this comment.
add a new property to
GitContextlooks complex...
GitContext is just an alias of GitHubContext and it's created from Context.constructor, make it very hard to add a new property, should I create some new interface types to resolve this?
There was a problem hiding this comment.
add a new property to
GitContextlooks complex...Just smth similar to https://github.com/docker/actions-toolkit/blob/f1593e3aa24d872bd4fa0a8f76a3bdaf26d20872/src/git.ts#L101-L103 is enough.
.sha is a existing property on Context(GitContext and GithubContext)
There was a problem hiding this comment.
I mean a new method for Git class in the actions toolkit like:
public static async commitDate(): Promise<Date> {
Then you would just need to consume it here when context is git.
src/meta.ts
Outdated
| this.version = this.getVersion(); | ||
| } | ||
|
|
||
| private getCommitDate(): Date { | ||
| const myOutput = execSync('git show -s --format="%ci" HEAD'); |
There was a problem hiding this comment.
Also we don't want to exec git when the context is workflow. You should just grab the commits.timestamp from the GitHub event for this case:
close #292