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

[#285] Bump text package after 2.0.1 (to 2.0.2) #283

Merged
merged 4 commits into from
Jun 23, 2023

Conversation

Martoon-00
Copy link
Member

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

Description

In the current master (after 2.0.1 version) text package changes implementation of pack, and this affects which rewrite rules should we use. After the new release, these and maybe some other changes will have to be applied.

Related issues(s)

Resolves #285

✓ 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

@gromakovsky
Copy link
Member

@Martoon-00 this PR won't be needed until text > 2.0.1 is released, did I get it right?

This was referenced Feb 21, 2023
@Martoon-00
Copy link
Member Author

Martoon-00 commented Mar 2, 2023

Right.

This information seems not actual at the moment of me answering though, since even text-2.0.2 has been released.

@Martoon-00
Copy link
Member Author

I'm not really sure whether commenting out the badges is what we want - when we make a release they won't be excluded from Hackage until the next release 🤔

Problem: after `2.0.1` version they rewrote `pack` completely, so now
our tricks with folding `pack`/`unpack`'s internals do not work.

For details, see [this commit in `text`
package](haskell/text@5a666e4).

Fortunately, annihilating entire `pack` and `unpack` calls is still an
option. Along with that `pack` rewrite they added `NOINLINE [0]` pragma
for `pack`, meaning that we have even better chances of the rewrite rule
to fire.

Solution: add a `MIN_VERSION` pragma to disable the now non-working
rewrite rule in the recent `text` versions.
Rely on the remaining `unpack (pack s) = s` and `pack (unpack s) = s`
rewrite rules to fire at stage 0.

Update maintenance notes, in the new reality verifying whether `text`
bump is correct should be simpler.
Problem: after the last commit it is safe to allow the recent
`text-2.0.2`.

Solution: allow it.
Problem: universum was excluded from the LTS, and xrefcheck complains on
badges containing invalid links.

Solution: comment those badges out, create a ticket for restoring the
things.
@Martoon-00 Martoon-00 force-pushed the martoon/bump-text-after-2.0.1 branch from a26f6ad to 70292d3 Compare June 23, 2023 14:00
@Martoon-00 Martoon-00 linked an issue Jun 23, 2023 that may be closed by this pull request
@Martoon-00 Martoon-00 marked this pull request as ready for review June 23, 2023 14:03
@Martoon-00 Martoon-00 changed the title Bump text package after 2.0.1 Bump text package after 2.0.1 (to 2.0.2) Jun 23, 2023
Copy link
Member

@gromakovsky gromakovsky left a comment

Choose a reason for hiding this comment

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

LGTM assuming you confirmed that benchmarks still give expected results.

Problem: after the large rewrite of `text` package (where it started
keeping UTF-8 instead of UTF-16) some things changed.

E.g. the code point mentioned in the comment is different now, and the
link to the `text`'s source code is not more actual.

Solution: strip the details that has to be maintained.

It all seems pretty clear without them.
@Martoon-00 Martoon-00 changed the title Bump text package after 2.0.1 (to 2.0.2) [#285] Bump text package after 2.0.1 (to 2.0.2) Jun 23, 2023
@Martoon-00 Martoon-00 merged commit be63573 into master Jun 23, 2023
@Martoon-00 Martoon-00 deleted the martoon/bump-text-after-2.0.1 branch June 23, 2023 15:04
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.

Allow text-2.0.2
3 participants