Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

API nit: standardize plurality of enums #228

Open
mcginty opened this issue Jan 18, 2022 · 2 comments
Open

API nit: standardize plurality of enums #228

mcginty opened this issue Jan 18, 2022 · 2 comments

Comments

@mcginty
Copy link
Contributor

mcginty commented Jan 18, 2022

Looking through the codebase I see a couple examples of enums where some are plural (FooAttrs) and some aren't (FooAttr):

rg 'enum .*Attr'

Might be good to settle on one in future breaking change updates (it looks like most projects choose the plural version).

@little-dude
Copy link
Owner

This is a good idea in theory, but in practice this is a breaking change which will force users to update their code for no gain in terms of features or bug fixing. For the record:

❯ rg 'enum .*Attr'
netlink-packet-generic/src/ctrl/nlas/mcast.rs
15:pub enum McastGrpAttrs {

ethtool/src/link_mode/attr.rs
57:pub enum EthtoolLinkModeAttr {

ethtool/src/coalesce/attr.rs
40:pub enum EthtoolCoalesceAttr {

netlink-packet-generic/src/ctrl/nlas/oppolicy.rs
54:pub enum OppolicyIndexAttr {

netlink-packet-generic/src/ctrl/nlas/policy.rs
99:pub enum NlPolicyTypeAttrs {

ethtool/src/ring/attr.rs
26:pub enum EthtoolRingAttr {

netlink-packet-generic/src/ctrl/nlas/mod.rs
25:pub enum GenlCtrlAttrs {

netlink-packet-generic/src/ctrl/nlas/ops.rs
15:pub enum OpAttrs {

ethtool/src/message.rs
59:pub enum EthtoolAttr {

ethtool/src/pause/attr.rs
25:pub enum EthtoolPauseStatAttr {
71:pub enum EthtoolPauseAttr {

ethtool/src/feature/attr.rs
55:pub enum EthtoolFeatureAttr {

netlink-packet-wireguard/src/nlas/device.rs
16:pub enum WgDeviceAttrs {

netlink-packet-wireguard/src/nlas/peer.rs
83:pub enum WgPeerAttrs {

netlink-packet-wireguard/src/nlas/allowedip.rs
16:pub enum WgAllowedIpAttrs {

@little-dude
Copy link
Owner

ethtool seems to use the singular version consistently. Changing all the type names may be a bit annoying for users, but we could probably do that incrementally: change the names, but keep aliases with a deprecation warning. netlink-generic-packet uses Attrs except for OppolicyIndexAttr. I think we could just change this one. netlink-packet-wireguard uses the plural version consistently.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants