Skip to content

[PERF] removing shadow variables#16198

Closed
miledxz wants to merge 1 commit intoprometheus:mainfrom
miledxz:cmd-prometheus-no-shadow
Closed

[PERF] removing shadow variables#16198
miledxz wants to merge 1 commit intoprometheus:mainfrom
miledxz:cmd-prometheus-no-shadow

Conversation

@miledxz
Copy link
Contributor

@miledxz miledxz commented Mar 11, 2025

The := is causing new variables for errors to be created, this PR is just an example for https://github.com/mmorel-35/prometheus/actions/runs/13772600546/job/38514471955?pr=408 fixes,

for example at some places like here:
https://github.com/miledxz/prometheus/blob/main/cmd/prometheus/main.go#L661 we just can't remove := as the funcs returns another variable, one of the workarounds is renaming err variable while keeping code clean and simple

Signed-off-by: miledxz <zedsprogramms@gmail.com>
@bboreham
Copy link
Member

Those ones look deliberate. Also not likely to help performance since they're run once at startup.

@miledxz
Copy link
Contributor Author

miledxz commented Mar 11, 2025

@bboreham thanks for fast review

@aknuds1
Copy link
Contributor

aknuds1 commented Apr 27, 2025

I don't see why it would be a good idea to ban shadowing of err in if statements as a rule. If you could actually show a performance improvement, that would be different.

@bboreham
Copy link
Member

I'd be ok changing code to eliminate shadowing and hence reduce scope for confusion, but in this particular case I would change the outer err not the inner one. The PR as it stands changes a variable in a wider scope, and that is worse.

@github-actions github-actions bot added the stale label Jun 27, 2025
@bboreham
Copy link
Member

Closing as unwanted.

@bboreham bboreham closed this Oct 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants