Update Prometheus vendoring some more, adapt Cortex #688
Conversation
juliusv
force-pushed
the
update-vendoring
branch
from
eb54e70 to
bfb5844
Compare
Gopkg.toml
Outdated
There was a problem hiding this comment.
Please, when you add a constraint or override, also add a comment explaining the reason why it is needed and the conditions under which it will be removed (e.g. "when PR 876 is released")
There was a problem hiding this comment.
Makes sense, though now I'm a bit stumped what exactly to write :P I just wanted something newer than the latest tagged Prometheus release in order to include some latest necessary changes, but then I thought it'd be good to update to latest master rather than just strictly the latest necessary commit. As in, it's a good idea for us to track upstream master quite closely for Prometheus anyway and regularly update. Do you think I should use a different kind of constraint, or just put a comment there like "Track upstream master closely, don't constrain to latest tagged release.".
There was a problem hiding this comment.
That doesn’t sound great over the long term. If we need a specific thing beyond the latest release we should be able to name it, and if it’s just to check compatibility or try out a new feature that could be on a branch (not master).
Maybe if we had releases for Cortex it would be easier to argue that master is always at the bleeding edge.
There was a problem hiding this comment.
branch = "master" would be a better way to express what you mean, as noted at prometheus/client_golang#375
There was a problem hiding this comment.
Damn, here I was all the time thinking I had put branch = "master" there already. Sorry! Of course that makes more sense. Changed.
There was a problem hiding this comment.
(or do you still think that generally tracking master isn't a good idea and I should rather vendor the latest specifically required commit, with a comment?)
There was a problem hiding this comment.
I'm fine with tracking master, but I still need you to write down the reason.
There are now 8 constraints in the file and I have no idea why any of them are there or when I can update them.
There was a problem hiding this comment.
Added a comment and will keep in mind for future constraints.
juliusv
force-pushed
the
update-vendoring
branch
from
a1ba2da to
41a2a7b
Compare
|
Rebased on latest master, which also had new vendoring changes. |
@jml One more little Prometheus vendoring update. Due to
depbehavior that I didn't understand previously (it will only update to tagged releases by default), it didn't update all the way to the version I needed in the last PR. But it's good to continuously keep up with upstream anyway :)