New release

[brancz]

Based on the discussion in #37 I looked through the commits since the v1.1 release, and the actual changes done to the library since that release seem to be:

• Variadic counter options: add variadic counterOpts funcs to server and client metrics constructor #25
• Allowing non global metrics registries: Non default registry #20

The one thing I'm struggling with is as we have never released the non global metrics registry code, I feel we should just remove the non-standard Register and MustRegister methods on the ClientMetrics and ServerMetrics structs.

I would suggest to remove the non standard methods and then do a v1.2 release, as there are no breaking changes.

@Bplotka @fengzixu

Let me know what you think, and I encourage everyone to double check my statement about backward compatibility to ensure we're not falsely pushing out a new minor version if it's not actually compatible.

Regarding your suggestions of linting errors @knweiss, we should probably look in detail at each of the failures, but I'd like to get the backward compatible changes out. Could you create a separate issue for the linting failures so we can have a look at them and decide separately?

[bwplotka]

Thanks for checking @brancz ! I double checked git history and yea - not that many were actually changed and not really any compatiblity break.

I feel we should just remove the non-standard Register and MustRegister

You mean this?

// Register registers all server metrics in a given metrics registry. Depending
// on histogram options and whether they are enabled, histogram metrics are
// also registered.
//
// Deprecated: ServerMetrics implements Prometheus Collector interface. You can
// register an instance of ServerMetrics directly by using
// prometheus.Register(m).
func (m *ServerMetrics) Register(r prom.Registerer) error {
	return r.Register(m)
}

// MustRegister tries to register all server metrics and panics on an error.
//
// Deprecated: ServerMetrics implements Prometheus Collector interface. You can
// register an instance of ServerMetrics directly by using
// prometheus.MustRegister(m).
func (m *ServerMetrics) MustRegister(r prom.Registerer) {
	r.MustRegister(m)
}

You are right, these methods were never released, so no compatiblity break. I agree, let's remove them.

Could you create a separate issue for the linting failures so we can have a look at them and decide separately?

Agree to look through lint issues each by each. Some are not straightforward to fix, so definitely (e.g package renames) will be quite distruptive, some are not really that important, others sound quite bad from the first glance TBH client.go:38:15:warning: error return value not checked
However, still maybe some can still be done in v1.2? I would give some deadline for the new release, if we can make it - nice. If not - we will release without some lint issue fixes. We are not in rush.

That being said, I think we are ok with just minor release. However before:

1. Let's set some release deadline (EOW, next EOW)?
2. Walk through current PRs - review them if still active and consider landing them for v1.2
3. Walk through lint issues and suggestions by @knweiss and @fengzixu

What do you think?

[brancz]

all of the above sounds good to me

[knweiss]

@brancz @Bplotka I've just created two PRs (#44 and #45) with some compatible cleanups for v1.2 as a start (I didn't have more time today).

[fengzixu]

@brancz @Bplotka Hi guys, i reread the code of out project and look #20 carefully.
I agree that we should remove the non-standard Register and MustRegister methods on the ClientMetrics and ServerMetrics structs. In my project of company, i will revise the use-pattern of prometheus registry. That's a good best practice of prometheus in go. 😀

For revise lint issues, i agree that we shuold handle it each by each. Because the lint rules are common, some lint suggestions don't approprate to our project. Just like @Bplotka said 「some are not really that important」. So, I suggest that we will release without some lint issue fixes. Even through we want fix it, we should do it in alone PR.

[brancz]

I merged #44 and #45 with the unproblematic linting errors and opened #46 to remove the deprecated functions.

What's left is that we need to go through the open PRs and see what we want/need to get down before the v1.2 release.

[brancz]

@knweiss and I went through some PRs and issues and closed/merged some. Everything outstanding is nothing we can't do in the future I would say. So after #46 we can do a release I would say.

[bwplotka]

Awesome guys, thanks! I reviewed all the outstanding things & merged #46 . I agree we are ready for a release now.

We can wait a little bit for any movement in pending PRs after review, but none of these are blocking release I would say.

[brancz]

I'm not in a hurry so I think we could wait a few more days. It would be nice if we can get #17 down, but certainly no show stopper.

Remaining outstanding issues are good candidates for future releases I would say.

[knweiss]

@brancz Please consider #47, too.

[sagikazarmark]

I'm not in a hurry

I agree, but I think if nothing blocks a release, waiting has no real gain: you can just do another release whenever you want.

[bwplotka]

v1.2.0 was releases (: