Skip to content

Cleanup in the core::fmt::num module #22916

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

Merged
merged 1 commit into from
Mar 3, 2015

Conversation

rprichard
Copy link
Contributor

  • Make num::UpperHex private. I was unable to determine why this struct
    is public. The num module itself is not public, and the UpperHex struct
    is not referenced anywhere in the core::fmt module. (Only the UpperHex
    trait is reference.) num::LowerHex is not public.

  • Remove the suffix parameters from the macros that generate integral
    display traits.

    The code to print the Debug::fmt suffixes was removed when Show was
    renamed to Debug. It was an intentional change. From RFC 0565:

    • Focus on the runtime aspects of a type; repeating information such
      as suffixes for integer literals is not generally useful since that
      data is readily available from the type definition.
  • Because Show was renamed to Debug, rename show! to debug!.

 * Make num::UpperHex private.  I was unable to determine why this struct
   is public.  The num module itself is not public, and the UpperHex struct
   is not referenced anywhere in the core::fmt module.  (Only the UpperHex
   trait is reference.)  num::LowerHex is not public.

 * Remove the suffix parameters from the macros that generate integral
   display traits.

   The code to print the Debug::fmt suffixes was removed when Show was
   renamed to Debug.  It was an intentional change.  From RFC 0565:

    * Focus on the *runtime* aspects of a type; repeating information such
      as suffixes for integer literals is not generally useful since that
      data is readily available from the type definition.

 * Because Show was renamed to Debug, rename show! to debug!.
@rust-highfive
Copy link
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

@rprichard rprichard changed the title Cleanup in the fmt::num module Cleanup in the core::fmt::num module Mar 1, 2015
@alexcrichton
Copy link
Member

@bors: r+ c1c02d9

Thanks!

@alexcrichton alexcrichton assigned alexcrichton and unassigned brson Mar 2, 2015
@bors
Copy link
Collaborator

bors commented Mar 3, 2015

⌛ Testing commit c1c02d9 with merge 1b67530...

Manishearth added a commit to Manishearth/rust that referenced this pull request Mar 3, 2015
…ichton

  * Make num::UpperHex private.  I was unable to determine why this struct
   is public.  The num module itself is not public, and the UpperHex struct
   is not referenced anywhere in the core::fmt module.  (Only the UpperHex
   trait is reference.)  num::LowerHex is not public.

 * Remove the suffix parameters from the macros that generate integral
   display traits.

   The code to print the Debug::fmt suffixes was removed when Show was
   renamed to Debug.  It was an intentional change.  From RFC 0565:

    * Focus on the *runtime* aspects of a type; repeating information such
      as suffixes for integer literals is not generally useful since that
      data is readily available from the type definition.

 * Because Show was renamed to Debug, rename show! to debug!.
@bors
Copy link
Collaborator

bors commented Mar 3, 2015

💔 Test failed - auto-linux-64-x-android-t

@bors bors merged commit c1c02d9 into rust-lang:master Mar 3, 2015
@rprichard rprichard deleted the fmt-num-cleanup branch March 13, 2015 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants