pkg/private/common: move IfIDType out of private/common and to its own package #4604
Conversation
Given that IfIDType represents generic interface IDs as used in segements distributed by the control service, it seems that it shouln't be private either, even if it currently can be. So the new package isn't private.
jiceatscion
requested review from
a team,
oncilla and
lukedirtwalker
as code owners
|
This change is |
There was no way to convince forbidigo that our strict naming rules for interface id (which explcitly forbid lower case "ifid") that it was ok for a package name.
There was a problem hiding this comment.
Reviewable status: 0 of 78 files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @lukedirtwalker)
pkg/segment/iface/iface.go line 26 at r2 (raw file):
// 64 bits. There is a lot of path-type-independent code that manipulates interface IDs without // interpreting them. This type is a container for them. type IfIDType uint64
At the usage site, this now is iface.IfIDType
It stutters, is barely readable, and full of redundancy.
How about we just name it ID? At usage site, this turns into iface.ID which is much easier on the eyes, and contains just as much information.
Suggestion:
type ID uint64
There was a problem hiding this comment.
Reviewable status: 0 of 78 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker and @oncilla)
pkg/segment/iface/iface.go line 26 at r2 (raw file):
Previously, oncilla (Dominik Roos) wrote…
At the usage site, this now is
iface.IfIDType
It stutters, is barely readable, and full of redundancy.How about we just name it
ID? At usage site, this turns intoiface.IDwhich is much easier on the eyes, and contains just as much information.
I agree this is much better. Done
There was a problem hiding this comment.
Reviewed 2 of 78 files at r1, 29 of 76 files at r2, 47 of 47 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker)
There was a problem hiding this comment.
Dismissed @oncilla from a discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @jiceatscion)
Given that IfIDType represents generic interface IDs as used in segments distributed by the control service, it seems that it shouldn't be private either, even if it currently can be. So the new package isn't private.
Fixes #4584