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

[Feature Request] Put full Git commit id in prometheus_build_info's revision label #1748

Closed
RichiH opened this Issue Jun 17, 2016 · 15 comments

Comments

Projects
None yet
7 participants
@RichiH
Copy link
Member

RichiH commented Jun 17, 2016

As per subject; the extra storage is neglible and as humans don't need to type the string, it's better to use longform.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jul 23, 2016

Does this have any real advantages? I can only see how it would make things like dashboards showing this information require more space.

@RichiH

This comment has been minimized.

Copy link
Member Author

RichiH commented Jul 23, 2016

Arguably, the UI should take care of UI considerations.

The backend should be as explicit as possible.

@juliusv juliusv added the help wanted label Jul 23, 2016

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Jul 23, 2016

Fine with me.

@vic3lord

This comment has been minimized.

Copy link
Contributor

vic3lord commented Jul 26, 2016

This is something that needs to be changed on prometheus/promu and not here, am I wrong?

@sdurrheimer

This comment has been minimized.

Copy link
Member

sdurrheimer commented Jul 26, 2016

promu is involved in this change yes, but we also might need to do changes in prometheus/common/version depending of the way of implementing it.

@vic3lord

This comment has been minimized.

Copy link
Contributor

vic3lord commented Jul 26, 2016

Let me know if I can do anything to help, really love this project I'd love to spend sometime on it even though it's a small thing

@sdurrheimer

This comment has been minimized.

Copy link
Member

sdurrheimer commented Jul 26, 2016

We can change the Revision prop here to store the full git revision.
But then we might need to change commandline outputs of the prometheus/common/version package here and here to display only the short version. I don't how the commandline output would look like with a full git revision.

@vic3lord

This comment has been minimized.

Copy link
Contributor

vic3lord commented Jul 26, 2016

We can add short-revision property and use both each when its needed

@sdurrheimer

This comment has been minimized.

Copy link
Member

sdurrheimer commented Jul 26, 2016

I don't think we need to add a prop to store a semi-duplicated data. We can computes it, since a short-revision corresponds to the first seven characters.

@vic3lord

This comment has been minimized.

Copy link
Contributor

vic3lord commented Jul 26, 2016

Great so just [:7] is enough

On 26 Jul 2016, at 15:19, Steve Durrheimer notifications@github.com wrote:

I don't think we need to add a prop to store a semi-duplicated data. We can computes it, since a short-revision corresponds to the first seven characters.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub #1748 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ACwA8MKfgymZJmPA9FKy6TYQR18PtqYZks5qZftegaJpZM4I4Jaa.

@grobie

This comment has been minimized.

Copy link
Member

grobie commented Jul 26, 2016

Nitpick, but git rev-parse --short HEAD is not guaranteed to return seven characters. That's the default minimum value of the --short flag, but in general git will return the shortest unambigious sha1 prefix. In theory these could also be more than 7 characters at some point.

@RichiH

This comment has been minimized.

Copy link
Member Author

RichiH commented Jul 26, 2016

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Jul 26, 2016

👍 on using the full one. When selecting along that dimension we are still free to do =~"abcdef12.*". No one loses really.

@sdurrheimer

This comment has been minimized.

Copy link
Member

sdurrheimer commented Aug 6, 2016

PR open on Promu's repo to make the change.

@lock

This comment has been minimized.

Copy link

lock bot commented Mar 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked and limited conversation to collaborators Mar 24, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.