fn Ok() breaks all code with use eyre::* in new releases

[JakkuSakura]

I noticed a breaking change with Ok(), meaning that all code like

match result {
  Ok(ok) => ...,
  Err(err) => ...
}

will break.

error[E0532]: expected tuple struct or tuple variant, found function `Ok`
   |
68 |                 Ok(rows) => rows,
   |                 ^^ not a tuple struct or tuple variant
   |
help: consider importing one of these items instead
   |
1  + use core::result::Result::Ok;
   |
1  + use pin_utils::core_reexport::result::Result::Ok;
   |
1  + use serde::__private::Ok;
   |
1  + use std::result::Result::Ok;
   |
     and 1 other candidate

This is not acceptable as Ok being a very common prelude item in Rust.

Even worse, the following code in the same file can't be ergonomic with fn Ok

let closure = || /* do something */ Ok(());

match result {
  Ok(ok) => ...,
  Err(err) => ...
}

[JakkuSakura]

#91

I suggest not breaking existing code. Maybe rename Ok() to EyreOk().
If I want to use eyre::*, it's funny to import something like use std::result::Result::Ok

[yaahc]

Hey, thank you for reporting this breakage. Our policy is to follow SemVer as defined by cargo¹, definitely not to break existing code. We appreciate and want to encourage users who report unexpected breakages when they encounter them.

That said, it's my understanding that adding new items is not considered a major change in the rust ecosystem and our usage of semver²³, even when it introduces breakage with globs.

We discussed this change in our team meeting today⁴ and considered the trade-offs of the proposed fix vs the current situation. One team member pointed out that the cargo team explicitly acknowledges glob imports as a forward compatibility hazard.

This is not considered a major change because conventionally glob imports are a known forwards-compatibility hazard. Glob imports of items from external crates should be avoided.

The main concern we discussed was API consistency; we already export other types with commonly used names such as Result and Report (aliased to Error!). Our unanimous⁵ preference was to provide a name that is consistent with existing APIs rather than provide the fix and expect users to write use eyre::EyreOk as Ok;. This also has the added benefit of not introducing yet another breaking change to update the name to fix the previous breakage with globs. I'm really sorry for the inconvenience this has caused, and thank you again for the issue and the PR; we really appreciate them.

Footnotes

1. https://doc.rust-lang.org/cargo/reference/semver.html ↩

2. https://doc.rust-lang.org/cargo/reference/semver.html#item-new ↩

3. https://predr.ag/blog/some-rust-breaking-changes-do-not-require-major-version/ ↩

4. https://hackmd.io/gpXlXuqMStKAMZExxCK_mg?view#2024-01-04 ↩

5. of the subset of team members present ↩

[yaahc]

All that said, I wanna be clear that I do not intend to shut down the discussion around the second concern you raised about the ergonomics of the API.

Even worse, the following code in the same file can't be ergonomic with fn Ok.

That seems like something we should still try to improve if we can or at least provide guidance for how to work around the issue.

[JakkuSakura]

Hi, thank you for your prompt discussion.

I understand that the PR tires to reflect changes 3 years ago in anyhow dtolnay/anyhow#192 (comment)

However, the original issue was not discussed at all because it seemed trivial, considering wildcard being forward compatible hazard. I hope I had engaged in the issue back then.

While removing or renaming in anyhow is already too late, it's still time to do something in eyre.

I think the final goal of this function is to type less keystrokes, not to cause problems or fatigue the end user. Compare
use eyre::;
std::Result::Ok()
and
use eyre::{Ok as EyreOk}; // to be useful
EyreOk()
and just
use eyre::;
EyreOk()

And consider their frequency in code

[thenorili]

If you have any other ergonomic improvements you'd like to suggest, please open a new issue! Compatibility breakages are expected from globbing as Jane mentioned. Renaming Eyre::Ok to Eyre::{Ok as EyreOk} is a valid way of importing.

When all else is equal, eyre prefers to maintain compatibility with anyhow. That's the main goal.