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

[#281] Allow text-2.0 #282

Merged
merged 3 commits into from Feb 21, 2023
Merged

[#281] Allow text-2.0 #282

merged 3 commits into from Feb 21, 2023

Conversation

Martoon-00
Copy link
Member

@Martoon-00 Martoon-00 commented Nov 22, 2022

Description

Increase the version upper bound on text package.

The related logic (pack, unpack) seems to be untouched, and the rewrite rule speeds up a call of toString . toText.

Related issues(s)

Fixed #281

✓ Checklist for your Pull Request

Ideally a PR has all of the checkmarks set.

If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).

  • I made sure my PR addresses a single concern, or multiple concerns which
    are inextricably linked. Otherwise I should open multiple PR's.
  • If I added/removed/deprecated functions/re-exports,
    I checked whether these changes impact the .hlint.yaml rules
    and updated them if needed.

Related changes (conditional)

  • Tests

    • If I added new functionality, I added tests covering it.
    • If I fixed a bug, I added a regression test to prevent the bug from
      silently reappearing again.
  • Documentation

    I checked whether I should update the docs and did so if necessary:

  • Record your changes

    • I added an entry to the changelog if my changes are visible to the users
      and
    • provided a migration guide for breaking changes if possible

Stylistic guide (mandatory)

  • My commit history is clean (only contains changes relating to my
    issue/pull request and no reverted-my-earlier-commit changes) and commit
    messages start with identifiers of related issues in square brackets.

    Example: [#42] Short commit description

    If necessary both of these can be achieved even after the commits have been
    made/pushed using rebase and squash.

✓ Release Checklist

  • I updated the version number in universum.cabal.
  • I updated the changelog and moved everything
    under the "Unreleased" section to a new section for this release version.
  • If any definitions (functions, type classes, instances, etc) were added,
    I added @since haddock annotations.
  • (After merging) I created a new entry in the releases page,
    with a summary of all user-facing changes.
    • I made sure a tag was created using the format vX.Y.Z

@Martoon-00 Martoon-00 changed the title Allow text-2.0 [#281] Allow text-2.0 Nov 22, 2022
@Swordlash
Copy link

Is there any progress here?

Martoon-00 and others added 3 commits February 16, 2023 12:30
Problem: the current `bgroupTextConversion` benchmarks turn out to be
unrepresentative now, they show the same (small) time disregard whether
do we have rewrite rules or not.

Probably this happened because in `text` they added some smart rewrite
rules on composition of `pack` with some other functions.
E.g. `Text.length . pack` is now rewritten to `List.length`.

Solution: replace the old benchmarks that used `length` with a trivial
benchmark that simply does `toText . toString`.

Notes:

Previously I preferred having relatively complex scenarios to
demonstrate that not only trivial `toText . toString` will get
short-circuited.

However, it's getting hard to setup a non-trivial case where our rewrite
rules would matter, and this is natural as `text` package tries to be
optimal in as many scenarios as possible. So soon it may happen that any
simple case of code will be optimal even without our own rewrite rule.
Problem: new major version of `text` has been released, and we do not
yet allow it.

Solution: increase the upper bound version of `text` to the most recent
`2.0.1` one.

I checked that rewrite rule is still working, actual, and affects the
benchmark.
Problem: changelog wasn't updated for 1.8.1 in
#271
Also we would like to make a new release to support text-2.something

Solution:
1. Add 1.8.1 changelog entry with the old change.
2. Add 1.8.1.1 with the new change. Note that in Haskell code
we only updated the benchmarks, changed one comment and
dependency bounds.
3. Update the package version.
@gromakovsky gromakovsky merged commit 1f840f0 into master Feb 21, 2023
@gromakovsky gromakovsky deleted the martoon/bump-text-2.0 branch February 21, 2023 08:53
@gromakovsky
Copy link
Member

@Swordlash thank you for the reminder, apparently I missed this PR or forgot about it.
I've just released https://hackage.haskell.org/package/universum-1.8.1.1 which supports text-2.0.1. text-2.0.2 will be supported after #283.

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.

Move to a newer version of text
4 participants