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.

Sign up for GitHub

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

Jump to bottom

log: type "level" is not used consistently (or not any more) #40972

Closed
knz opened this issue Sep 23, 2019 · 0 comments · Fixed by #47923
Closed

log: type "level" is not used consistently (or not any more) #40972

knz opened this issue Sep 23, 2019 · 0 comments · Fixed by #47923
Assignees
@knz
Labels
A-logging In and around the logging infrastructure. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.

Comments

@knz
Copy link
Contributor

knz commented Sep 23, 2019

The log package originally had an exported type Level used by log.V().
Somewhere during history the external interface of log.V() and related functions was changed to accept an int32 directly, and the type Level was un-exported.

It is still used internally to the log package but its value is unclear, given that half of the code now uses level and the other half uses int32.

The code should be simplified to either use int32 everywhere, or define type Level = int32 and rewrite the code to use Level without making callers unhappy.

cc @tbg
@knz knz added the C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. label Sep 23, 2019
@knz knz self-assigned this Sep 23, 2019
@knz knz added the A-logging In and around the logging infrastructure. label Sep 23, 2019
@knz knz mentioned this issue Sep 23, 2019
Merged
craig bot pushed a commit that referenced this issue Oct 17, 2019
@knz @jordanlewis
Merge #40993 #41654
0e655a3 
40993: util/log: refactor and bug fixes r=knz a=knz

Fixes #40983.
Fixes #40973.
Fixes #40974.
Fixes #41231.

I might also want to add code to clean up #40972, #40982 and #40990 provided I receive some guidance from @tbg or someone familiar with the log package.


41654: parser: fix telemetry link for materialized views r=jordanlewis a=jordanlewis

This commit fixes the error message that you get if you try to create a
materialized view to point at the right GitHub issue.

Release note: None

Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>
Co-authored-by: Jordan Lewis <jordanthelewis@gmail.com>
@knz knz mentioned this issue Apr 22, 2020
Merged
@craig craig bot closed this as completed in 2633c35 Apr 22, 2020
@craig craig bot closed this as completed in #47923 Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@knz knz

Labels
A-logging In and around the logging infrastructure. C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

util/log: type the Level argument to vmodule functions as own type knz/cockroach
1 participant
@knz