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

Merge empty_error #9000

Merged
merged 10 commits into from
Feb 9, 2024
Merged

Merge empty_error #9000

merged 10 commits into from
Feb 9, 2024

Conversation

AdRiley
Copy link
Member

@AdRiley AdRiley commented Feb 7, 2024

Pull Request Description

We don't need 3 copies of empty_error. This PR makes them one.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@AdRiley AdRiley added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 7, 2024
Copy link
Member

@jdunkerley jdunkerley left a comment

Choose a reason for hiding this comment

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

A few stylist in tweaks but looks good. And only one type makes me happy.

@AdRiley AdRiley added the CI: Ready to merge This PR is eligible for automatic merge label Feb 8, 2024
@AdRiley AdRiley force-pushed the wip/adr/merge-empty-errors branch from 8981b53 to 1091260 Compare February 8, 2024 18:47
@AdRiley AdRiley force-pushed the wip/adr/merge-empty-errors branch from 3ed2983 to 3769c95 Compare February 9, 2024 12:19
@mergify mergify bot merged commit 1dc8c1c into develop Feb 9, 2024
26 checks passed
@mergify mergify bot deleted the wip/adr/merge-empty-errors branch February 9, 2024 14:18
@@ -710,7 +710,7 @@ type Array

[0, 1, 2].to_array . reduce (+)
reduce : (Any -> Any -> Any) -> Any -> Any
reduce self function ~if_empty=(Error.throw Empty_Error) = Vector.reduce self function if_empty
reduce self function ~if_empty=(Error.throw (Empty_Error.Error Vector)) = Vector.reduce self function if_empty
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be

Suggested change
reduce self function ~if_empty=(Error.throw (Empty_Error.Error Vector)) = Vector.reduce self function if_empty
reduce self function ~if_empty=(Error.throw (Empty_Error.Error Array)) = Vector.reduce self function if_empty

given that we are operating with an Array here??

I know they are pretty much interchangeable but they still are separate types.

(Sorry I just noticed it after getting a merge conflict with #8867)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Good spot :) I will change that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants