-
Notifications
You must be signed in to change notification settings - Fork 119
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 tz attribute to handlebar date function #251
Conversation
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.
Don't think an extra timezone
attribute makes sense as it should only be assigned to the date
handlebar function (see my suggestion below).
src/meta.ts
Outdated
@@ -130,7 +130,7 @@ export class Meta { | |||
const vraw = this.setValue( | |||
handlebars.compile(tag.attrs['pattern'])({ | |||
date: function (format) { | |||
return moment(currentDate).utc().format(format); | |||
return moment(currentDate).tz(tag.attrs['timezone']).format(format); |
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.
Instead of an extra attribute it would be better to add an hash arg to the handlebar func like: https://handlebarsjs.com/guide/expressions.html#helpers-with-hash-arguments
date: function (format, options) {
const m = moment(currentDate);
Object.keys(options.hash).forEach(key => {
switch (key) {
case "tz":
m.tz(options.hash[key]);
break;
default:
throw new Error(`Unknown ${key} attribute`);
}
})
return m.format(format);
}
# no tz
type=schedule,pattern={{date 'YYYYMMDD-hhmmss'}}
# with tz
type=schedule,pattern={{date 'YYYYMMDD-hhmmss' tz='Asia/Tokyo'}}
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.
@crazy-max Thanks for your review. I have updated.
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.
Can you also update the example in global expression section of the README: https://github.com/docker/metadata-action#date-format ?
Done it ! |
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.
Thanks! Can you squash your commits and update the commit message with something like:
Add tz attribute to handlebar date function
Signed-off-by: chroju <chroju@users.noreply.github.com>
squashed and forced to push 👍 |
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.
LGTM thanks!
Fixes #250 . Add
timezone
parameters totype=schedule
andtype=raw
.