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.

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

Audit API naming #871

Open
9 of 13 tasks
Tracked by #671
joshlf opened this issue Feb 13, 2024 · 4 comments
Open
9 of 13 tasks
Tracked by #671

Audit API naming #871

joshlf opened this issue Feb 13, 2024 · 4 comments
Assignees
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking

Comments

@joshlf
Copy link
Member

joshlf commented Feb 13, 2024

Progress

Details

In the same vein as #253, we should audit our API for:

  • Internal consistency
  • Consistency with existing conventions

Also see #284, the "Naming" section of #1095

@joshlf joshlf added the compatibility-breaking Changes that are (likely to be) breaking label Feb 13, 2024
@joshlf joshlf mentioned this issue Feb 13, 2024
65 tasks
@jswrenn
Copy link
Collaborator

jswrenn commented Apr 17, 2024

I have a scratchpad of name correspondences here: https://docs.google.com/spreadsheets/d/1o_ll7S3iW7xe3UdG_t8hzjBmUD-uRHXKyxZXJ7i1D2g/edit?usp=sharing

We do have some minor naming inconsistencies between Ref and FromBytes. For example, FromBytes::{mut_from_prefix,ref_from_prefix} corresponds to Ref::new_from_prefix. The pattern here, which holds for many other such methods, is that one drops the mut/ref and replaces it with new.

This pattern does not apply to FromBytes::{mut_from,ref_from}. The pattern suggests a correspondence to Ref::new_from, but it actually corresponds to Ref::new.

As a point of brevity: I wonder if we should drop the new_ prefixes. They're abundant and redundant — the fact that these are constructed a Ref is already denoted by from.

The placement of unaligned and zeroed in the Ref names is awkward. Why new_slice_unaligned_from_prefix and not new_unaligned_slice_from_prefix (i.e., adjective preceding the noun)?

@joshlf
Copy link
Member Author

joshlf commented Apr 23, 2024

Also: When we add KnownLayout APIs in #996, we need to bikeshed their names too.

@joshlf joshlf changed the title Audit API naming for consistency with stdlib and general Rust patterns Audit API naming May 7, 2024
joshlf added a commit that referenced this issue May 7, 2024
Add `ref_` prefix to `from_{prefix,suffix}_with_trailing_elements` to be
consistent with other methods.

Makes progress on #871
joshlf added a commit that referenced this issue May 7, 2024
Add `ref_` prefix to `from_{prefix,suffix}_with_trailing_elements` to be
consistent with other methods.

Makes progress on #871
joshlf added a commit that referenced this issue May 7, 2024
Add `ref_` prefix to `from_{prefix,suffix}_with_trailing_elements` to be
consistent with other methods.

Makes progress on #871
github-merge-queue bot pushed a commit that referenced this issue May 7, 2024
Add `ref_` prefix to `from_{prefix,suffix}_with_trailing_elements` to be
consistent with other methods.

Makes progress on #871
@jswrenn
Copy link
Collaborator

jswrenn commented May 10, 2024

Re. the count-taking methods: Current contenders include _with_trailing_elements and _with_count. These result in identifiers of up to 39 characters long (mut_from_prefix_with_trailing_elements) and 27 characters long (mut_from_prefix_with_count), respectively.

Personally, I'm inclined towards the briefer option. I'd also argument that with_trailing_elements is a misnomer; these methods don't consume trailing elements, they consume a count of the trailing elements.

For even more bevity, we could even do:

$ echo {,count_}{mut,ref}_from{,_prefix,_suffix} | tr ' ' '\n' 
mut_from_prefix
mut_from_suffix
ref_from
ref_from_prefix
ref_from_suffix
count_mut_from
count_mut_from_prefix
count_mut_from_suffix
count_ref_from
count_ref_from_prefix
count_ref_from_suffix

The longest identifier here is only 22 characters long. Personally, I'm a little attached to having all of our identifiers on FromBytes start with either ref or mut.

@joshlf
Copy link
Member Author

joshlf commented May 10, 2024

Discussed offline, and decided to go with xxx_with_elems for methods that take a trailing slice element count.

joshlf added a commit that referenced this issue May 10, 2024
joshlf added a commit that referenced this issue May 10, 2024
github-merge-queue bot pushed a commit that referenced this issue May 10, 2024
joshlf added a commit that referenced this issue May 11, 2024
In `FromBytes` method names, replace `with_trailing_elements` with
`with_elems`.

Makes progress on #871
joshlf added a commit that referenced this issue May 11, 2024
In `FromBytes` method names, replace `with_trailing_elements` with
`with_elems`.

Makes progress on #871
joshlf added a commit that referenced this issue May 11, 2024
This commit makes some progress, but more remain to be added.

Makes progress on #871
github-merge-queue bot pushed a commit that referenced this issue May 11, 2024
In `FromBytes` method names, replace `with_trailing_elements` with
`with_elems`.

Makes progress on #871
github-merge-queue bot pushed a commit that referenced this issue May 11, 2024
This commit makes some progress, but more remain to be added.

Makes progress on #871
jswrenn added a commit that referenced this issue May 16, 2024
jswrenn added a commit that referenced this issue May 16, 2024
jswrenn added a commit that referenced this issue May 16, 2024
jswrenn added a commit that referenced this issue May 16, 2024
joshlf added a commit that referenced this issue May 17, 2024
...to `ref_from_bytes[_with_elems]`. This brings the names in line with
the `Ref` methods of the same names, and avoids the awkward name
`ref_from_with_elems`.

Similarly, rename `Ref::unaligned_from` to `unaligned_from_bytes`. We
had already renamed `unaligned_from_with_elems` to
`unaligned_from_bytes_with_elems`.

Makes progress on #871
joshlf added a commit that referenced this issue May 17, 2024
...to `ref_from_bytes[_with_elems]`. This brings the names in line with
the `Ref` methods of the same names, and avoids the awkward name
`ref_from_with_elems`.

Similarly, rename `Ref::unaligned_from` to `unaligned_from_bytes`. We
had already renamed `unaligned_from_with_elems` to
`unaligned_from_bytes_with_elems`.

Makes progress on #871
joshlf added a commit that referenced this issue May 17, 2024
...to `ref_from_bytes[_with_elems]`. This brings the names in line with
the `Ref` methods of the same names, and avoids the awkward name
`ref_from_with_elems`.

Similarly, rename `Ref::unaligned_from` to `unaligned_from_bytes`. We
had already renamed `unaligned_from_with_elems` to
`unaligned_from_bytes_with_elems`.

Makes progress on #871
github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
...to `ref_from_bytes[_with_elems]`. This brings the names in line with
the `Ref` methods of the same names, and avoids the awkward name
`ref_from_with_elems`.

Similarly, rename `Ref::unaligned_from` to `unaligned_from_bytes`. We
had already renamed `unaligned_from_with_elems` to
`unaligned_from_bytes_with_elems`.

Makes progress on #871
joshlf added a commit that referenced this issue May 17, 2024
- Rename some non-deprecated `FromBytes` methods:
  - `mut_from` -> `mut_from_bytes`
  - `mut_from_with_elems` -> `mut_from_bytes_with_elems`
  - `read_from` -> `read_from_bytes`
- Add missing deprecated `Ref` methods:
  - `new_unaligned`
  - `new_unaligned_from_prefix`
  - `new_unaligned_from_suffix`
- In `FromBytes`, coalesce deprecated methods at the end of the trait
- Update names in deprecation messages (some names were stale)
- Make all deprecated methods return `Option`s rather than `Result`s,
  consistent with their equivalents on 0.7
- Mark all deprecated methods as `#[inline(always)]`

Makes progress on #871
github-merge-queue bot pushed a commit that referenced this issue May 17, 2024
- Rename some non-deprecated `FromBytes` methods:
  - `mut_from` -> `mut_from_bytes`
  - `mut_from_with_elems` -> `mut_from_bytes_with_elems`
  - `read_from` -> `read_from_bytes`
- Add missing deprecated `Ref` methods:
  - `new_unaligned`
  - `new_unaligned_from_prefix`
  - `new_unaligned_from_suffix`
- In `FromBytes`, coalesce deprecated methods at the end of the trait
- Update names in deprecation messages (some names were stale)
- Make all deprecated methods return `Option`s rather than `Result`s,
  consistent with their equivalents on 0.7
- Mark all deprecated methods as `#[inline(always)]`

Makes progress on #871
@joshlf joshlf added the blocking-next-release This issue should be resolved before we release on crates.io label May 30, 2024
jswrenn added a commit that referenced this issue Jul 1, 2024
- `try_mut_from` -> `try_mut_from_bytes`
- `try_read_from` -> `try_read_from_bytes`
- `try_ref_from` -> `try_ref_from_bytes`

Makes progress towards #871
github-merge-queue bot pushed a commit that referenced this issue Jul 1, 2024
- `try_mut_from` -> `try_mut_from_bytes`
- `try_read_from` -> `try_read_from_bytes`
- `try_ref_from` -> `try_ref_from_bytes`

Makes progress towards #871
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking-next-release This issue should be resolved before we release on crates.io compatibility-breaking Changes that are (likely to be) breaking
Projects
None yet
Development

No branches or pull requests

2 participants