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

Rename fail! to panic! #17894

Merged
merged 2 commits into from
Oct 29, 2014
Merged

Rename fail! to panic! #17894

merged 2 commits into from
Oct 29, 2014

Conversation

steveklabnik
Copy link
Member

This in-progress PR implements #17489.

I made the code changes in this commit, next is to go through alllllllll the documentation and fix various things.

  • Rename column headings as appropriate, # Panics for panic conditions and # Errors for Results.
  • clean up usage of words like 'fail' in error messages

Anything else to add to the list, @aturon ? I think I should leave the actual functions with names like slice_or_fail alone, since you'll get to those in your conventions work?

I'm submitting just the code bits now so that we can see it separately, and I also don't want to have to keep re-building rust over and over again if I don't have to 😉

Listing all the bits so I can remember as I go:

  • compiler-rt
  • compiletest
  • doc
  • driver
  • etc
  • grammar
  • jemalloc
  • liballoc
  • libarena
  • libbacktrace
  • libcollections
  • libcore
  • libcoretest
  • libdebug
  • libflate
  • libfmt_macros
  • libfourcc
  • libgetopts
  • libglob
  • libgraphviz
  • libgreen
  • libhexfloat
  • liblibc
  • liblog
  • libnative
  • libnum
  • librand
  • librbml
  • libregex
  • libregex_macros
  • librlibc
  • librustc
  • librustc_back
  • librustc_llvm
  • librustdoc
  • librustrt
  • libsemver
  • libserialize
  • libstd
  • libsync
  • libsyntax
  • libterm
  • libtest
  • libtime
  • libunicode
  • liburl
  • libuuid
  • llvm
  • rt
  • test

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@aturon
Copy link
Member

aturon commented Oct 9, 2014

@steveklabnik Thanks for getting started on this! Another item to add to the list: renaming other uses of fail in function and parameter names. This is less important, but it'd be good to do and probably easiest as part of the same pass.

@steveklabnik
Copy link
Member Author

I am unsure of the changes in libcollections/vec.rs, I think that's talking about panic? But not 100% sure.

@steveklabnik
Copy link
Member Author

Whew, rebased. That sucked.

@Gankra
Copy link
Contributor

Gankra commented Oct 20, 2014

Everything in vec looks good to me

@steveklabnik
Copy link
Member Author

@aturon i'm guessing that that would be 'deprecate and make a new one'? Like the slice_or_fail methods in libcollections, for example?

@steveklabnik
Copy link
Member Author

Unsure how to rephrase

//! The core library cannot define failure, but it does *declare* failure. This

@steveklabnik
Copy link
Member Author

I guess libcore's failure isn't really the same thing? it's like, hardcore failure, right?

@steveklabnik
Copy link
Member Author

(speaking of src/libcore/failure.rs specifically)

@steveklabnik
Copy link
Member Author

checking off libsemver because it's already in an external crate

@steveklabnik
Copy link
Member Author

same for all the other ones which were moved out

@aturon
Copy link
Member

aturon commented Oct 21, 2014

@steveklabnik

@aturon i'm guessing that that would be 'deprecate and make a new one'? Like the slice_or_fail methods in libcollections, for example?

I'm not sure if something got lost in github, but it's not clear what this is referring to?

@aturon
Copy link
Member

aturon commented Oct 21, 2014

@steveklabnik

Unsure how to rephrase

   //! The core library cannot define failure, but it does *declare* failure. This

The "failure" here is referring to the implementation of fail!, so this should be talking about panicking instead (and the file should be renamed as well.)

@aturon
Copy link
Member

aturon commented Oct 21, 2014

Oh, and yes, it's fine to focus on doc comments and leave actual function/parameter names to API stabilization.

@steveklabnik
Copy link
Member Author

Yeah, I was talking about the method and parameter names.

@steveklabnik
Copy link
Member Author

Whew! Getting there! I've knocked out all the ones that have no messages, so the ones that left all need work.

@steveklabnik
Copy link
Member Author

aaaand rebased

@aturon
Copy link
Member

aturon commented Oct 27, 2014

This is amazing... please ping me on IRC when you're done and I'll try to do the final review quickly to avoid further rebasing pain.

@steveklabnik
Copy link
Member Author

No worries, after the rebase involving the removal of libdebug (imagine how many format strings went from {:?} to {}) it's not been that big of a deal :)

@steveklabnik
Copy link
Member Author

I'm guessing i'll be done in a day or two. been working on other stuff too since this is kind of mind-numbing.

@steveklabnik
Copy link
Member Author

std::task::failing should probably get re-named with the conventions update, @aturon

@steveklabnik
Copy link
Member Author

In the libcore stuff, I did change the name of the implementation of panic. I left the others alone though /cc @aturon

@steveklabnik
Copy link
Member Author

Okay! This is ready for a preliminary review, @aturon .

This may not currently compile, I haven't been able to test because I'm literally waiting for the internet to be installed in my new place within the next few hours, and I'm not going to download a snapshot over tethering 😉 I'll be able to double check that this actually works once that's set up.

@steveklabnik
Copy link
Member Author

This now compiles and stuff correctly. Woo!

@aturon
Copy link
Member

aturon commented Oct 28, 2014

OK, this looks basically good, but I've commented on a bunch of places where I think "fail" or "failure" is actually the right term.

After that, please squash down to a couple commits, and add a [breaking-change] marker along with instructions for how people should update their code.

Thanks again!

burtonageo added a commit to burtonageo/genmesh that referenced this pull request Oct 30, 2014
ghost pushed a commit to gfx-rs/genmesh that referenced this pull request Oct 30, 2014
Changed fail macro to panic (compatibility with rust-lang/rust#17894)
steveklabnik added a commit to steveklabnik/rust-mustache that referenced this pull request Nov 4, 2014
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.

8 participants