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.

Sign up for GitHub

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

Jump to bottom

Manually define sconcat to use the efficient concat function #142

Merged
merged 1 commit into from
May 11, 2020
Merged

Manually define sconcat to use the efficient concat function #142

merged 1 commit into from
May 11, 2020

Conversation

gwils
Copy link
Contributor

@gwils gwils commented Nov 30, 2017

No description provided.

@hvr
Copy link
Member

hvr commented Feb 16, 2018

Nice

  1. Does semigroups which provides instances for older GHCs need this as well?
  2. Can you please write a changelog entry; something short pointing out "optimised sconcat implementation for bytestring (Manually define sconcat to use the efficient concat function #142)" would be enough?

@gwils gwils mentioned this pull request Feb 20, 2018
Merged
@gwils
Copy link
Contributor Author

gwils commented Feb 20, 2018

  1. Explicitly define sconcat on ByteString to use Data.ByteString.concat ekmett/semigroups#85
  2. Done

@hvr
Copy link
Member

hvr commented Feb 23, 2019

@gwils do you mind rebasing this? :-)

@gwils
Copy link
Contributor Author

gwils commented Mar 4, 2019

Done. This is no longer blocked by merge conflicts :)

sjakobi
sjakobi approved these changes Dec 18, 2019
View reviewed changes
Copy link
Member

@sjakobi sjakobi left a comment

Choose a reason for hiding this comment

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

Nice! 👍

I hope this gets merged soon, once the new merge conflict has been resolved! :)

@Bodigrim
Copy link
Contributor

@gwils could you please-please-please rebase it once more? ;)

@gwils
Copy link
Contributor Author

gwils commented May 11, 2020

Done. You'd better merge it quickly, otherwise I'll be back here to rebase it again in 2021 :)

@gwils
Copy link
Contributor Author

gwils commented May 11, 2020

Pushed again to hopefully fix CI...

Bodigrim
Bodigrim approved these changes May 11, 2020
View reviewed changes
Copy link
Contributor

@Bodigrim Bodigrim left a comment

Choose a reason for hiding this comment

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

Looks good to me! AFAIU @hvr's comments above, it's approved from his side as well. @cartazio care to merge, please?

@cartazio cartazio merged commit bac4225 into haskell:master May 11, 2020
@gwils gwils deleted the sconcat branch May 14, 2020 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bodigrim Bodigrim Bodigrim approved these changes

@sjakobi sjakobi sjakobi approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@gwils @hvr @Bodigrim @sjakobi @cartazio