Skip to content

Conversation

JacquesCarette
Copy link
Contributor

Many many were actually closer to needed, so left alone.

Some with were made clearer (to be merely pattern matches, but ones that caused various things to reduce, so with was the better idiom to use).

Basically the only ones that were not touched were the ones in Data.List.Base !!

Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

I'm basically fine with this, but am not convinced that the case_of_ versions are an improvement on balance, because of their verbosity relative to using irrefutable with.

But I'm willing to be persuaded otherwise!

@jamesmckinna jamesmckinna changed the title A number of 'with' are not really needed A number of with are not really needed Apr 15, 2024
Copy link
Contributor

@jamesmckinna jamesmckinna left a comment

Choose a reason for hiding this comment

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

Some (further) specimen comments, mostly stylistic, with no mandatory changes, but I would (at least!) consider them...

@JacquesCarette
Copy link
Contributor Author

Considered all, I'll enact about 2/3 of them.

@MatthewDaggitt MatthewDaggitt added this to the v2.1 milestone Apr 20, 2024
@jamesmckinna
Copy link
Contributor

Looks good!
can we have a second review from someone please?

JacquesCarette and others added 3 commits April 23, 2024 07:18
layout for readability

Co-authored-by: G. Allais <[email protected]>
layout for readability

Co-authored-by: G. Allais <[email protected]>
layout for readability

Co-authored-by: G. Allais <[email protected]>
@JacquesCarette
Copy link
Contributor Author

Thanks for the suggestions @gallais those did noticeably improve readability.

@jamesmckinna jamesmckinna added this pull request to the merge queue Apr 23, 2024
Merged via the queue into master with commit 7c11a0e Apr 23, 2024
@JacquesCarette JacquesCarette deleted the WithBeGone branch April 23, 2024 23:26
@andreasabel andreasabel mentioned this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants