replace level pkg

[royeo]

filter log calls by log level

Fixes #384

[matthiasr]

This is more thorough than I expected! I like it, the only thing I would like to clarify are the various messages we show users:

When providing an invalid log level, state what the valid levels are.

Every test error should have a unique text so it is clear which assertion failed. For the table tests, also include which input failed. When checking for errors, say so ("expected no error setting log level to %s, got: %s") instead of "expected: nil".

[royeo]

If an invalid log level is provided, the program will report an error.

I have modified the test code, please continue to review @matthiasr

[royeo]

Please review this PR @matthiasr

[matthiasr]

Sorry for the delay, I'll review again this week

…

On Sun, Aug 22, 2021, 21:12 Roy ***@***.***> wrote: @royeo <https://github.com/royeo> requested your review on: #385 <#385> replace level pkg. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#385 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABAEBXH72L4E66OQKZ23DDT6FDYZANCNFSM5CBR7OTA> .

[royeo]

How's it going?

[SuperQ]

LGTM. I would love if this optimization was pushed upstream.

[royeo]

Is MR ready to merge? @matthiasr

[matthiasr]

Yep, looks good, thank you! I am not super happy about try --help as a call to action when specifying an invalid level, but I don't see an elegant way to change that either.

[matthiasr]

Ooops, sorry, I need to back this out. The tests didn't run on this at all for some reason, except for the DCO check, which made everything look green.

[matthiasr]

@royeo unfortunately the new files are missing the mandatory license header. Also, the tests didn't run on this PR at all, possibly because you opened it from your main branch. Could you please open a new PR, with headers like

// Copyright 2021 The Prometheus Authors
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

on both of the new files?

[matthiasr]

Please create a branch just for this change and create the PR from that.

[royeo]

@matthiasr I create a new PR #390, please review it