Skip to content

Conversation

Vrtgs
Copy link

@Vrtgs Vrtgs commented Jun 19, 2025

add initial support for f16 and f128, next steps are to add this support in serde_test if this lands

helps with adding ecosystem support par in rust-lang/rust#116909

@Vrtgs Vrtgs requested a review from Mingun June 21, 2025 12:20
@@ -1439,6 +1463,17 @@ pub trait Visitor<'de>: Sized {
))
}

/// The input contains a `f16`.
///
/// The default implementation forwards to visit_f64.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the others:

Suggested change
/// The default implementation forwards to visit_f64.
/// The default implementation forwards to [`visit_f64`].

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this break documentation build sadly

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to add something like:

[`visit_f64`]: #method.visit_f64

at the bottom of the doc comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very strange, that documentation is broken, it should be broken in other places as well. The correct link should be

[`visit_f64`](Self::visit_f64)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either way should work. You can see that it does link correctly on docs.rs for the other links for this trait. For example: https://docs.rs/serde/latest/serde/de/trait.Visitor.html#method.visit_u64

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course, just intralinks can be checked by the compiler

Copy link
Contributor

@Mingun Mingun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to roll back the removal of integer128, the maintainers will not approve it.

My reviews means nothing, I do not have any rights in this repository. So probably request my reviews is pointless...

@bioinformatist
Copy link

@Vrtgs Any news for this?

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

Successfully merging this pull request may close these issues.

4 participants