Skip to content
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

feat: add time template helpers #627

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

freak12techno
Copy link

@freak12techno freak12techno commented Apr 29, 2024

Copied from here https://github.com/prometheus/prometheus/blob/34ee8c607809cca973a1d7383713035045f681d8/template/template.go#L267, the eventual goal is to reuse it in Alertmanager as well to allow it as a template helper, and also in Prometheus.

Related: prometheus/alertmanager#3717
Related: prometheus/alertmanager#3720 (comment)

Function itself is moved as is, tests are updated as there's no templates rendering here.

Also added the .idea to .gitignore (as I use Goland IDE and it creates some files that are probably not to be committed) and added the go.mod entry to go.mod and go.sum (otherwise make test would fail).

@freak12techno freak12techno force-pushed the add-time-template-helpers branch 3 times, most recently from a629df0 to ca27390 Compare April 29, 2024 21:37
@freak12techno
Copy link
Author

@gotjosh can you look at this?

.gitignore Outdated Show resolved Hide resolved
@beorn7 beorn7 requested a review from gotjosh May 8, 2024 17:31
Signed-off-by: Sergey <freak12techno@gmail.com>
Signed-off-by: Sergey <freak12techno@gmail.com>
Signed-off-by: Sergey <freak12techno@gmail.com>
Signed-off-by: Sergey <freak12techno@gmail.com>
@LMantovan
Copy link

LMantovan commented May 15, 2024

Hi @freak12techno, in the old PR prometheus/alertmanager#3720 you also included time.since. Is it possible to add here?

@freak12techno
Copy link
Author

freak12techno commented May 16, 2024

@LMantovan I'll still need to make a PR towards alertmanager to use this template, my plan is to add time.Since in the next PR which will supersede the one I closed.

This one needs to be merged first regardless though.

@freak12techno
Copy link
Author

@gotjosh ping?

@beorn7
Copy link
Member

beorn7 commented May 22, 2024

@gotjosh are you up to reviewing this? Either way, could you comment so that we at least know you have seen this PR?

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.

None yet

3 participants