Skip to content

Pull request for issue #19#20

Closed
google-labs-jules[bot] wants to merge 1 commit into
mainfrom
feature/date-time-variables
Closed

Pull request for issue #19#20
google-labs-jules[bot] wants to merge 1 commit into
mainfrom
feature/date-time-variables

Conversation

@google-labs-jules

Copy link
Copy Markdown
Contributor

Fixes #19

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR adds new date and time variables for webhooks. The implementation is correct, but there is a minor performance issue due to multiple calls to Date and Date.now(). I've added a comment with a suggestion to refactor this for better performance and consistency.

Comment thread popup/popup.js
Comment on lines +178 to 181
"{{currentUnixTimestampMiliseconds}}": Date.now(),
"{{currentIsoDate}}": new Date().toISOString().slice(0, 10),
"{{currentIsoDateTime}}": new Date().toISOString(),
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low: For efficiency and consistency, it's better to avoid multiple calls to Date.now() and new Date(). You can get the timestamp and ISO string once and reuse them.

A more efficient way to write this would be to declare now and isoNow variables before the placeholders object.

const now = Date.now();
const isoNow = new Date(now).toISOString();
//...
"{{currentUnixTimestamp}}": Math.floor(now / 1000),
"{{currentUnixTimestampMiliseconds}}": now,
"{{currentIsoDate}}": isoNow.slice(0, 10),
"{{currentIsoDateTime}}": isoNow,

This would require refactoring the surrounding code slightly, but would improve performance and make the code cleaner.

@cmuench cmuench closed this Aug 7, 2025
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.

Support DateTime specific variables

1 participant