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

#8975, #8981, #8983 rebased #9051

Merged
merged 5 commits into from
Jun 22, 2023
Merged

#8975, #8981, #8983 rebased #9051

merged 5 commits into from
Jun 22, 2023

Conversation

ffaf1
Copy link
Collaborator

@ffaf1 ffaf1 commented Jun 20, 2023

#8975, #8981 and #8983, cherry picked.


Please include the following checklist in your PR:

Bonus points for added automated tests!

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Thanks a lot. That preserve commit history, so it's better than just squashing.

Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

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

Terrific!

You may need to be careful to not reserve the branch name phadej-rebased — we may need it in the future 😉

@ulysses4ever
Copy link
Collaborator

We should not forget to close the base PRs when this one is merged...

@ulysses4ever
Copy link
Collaborator

While we're on it... @ffaf1 how do you feel about including #8983 here (whatever is left from it after you handled the other two)?

@ffaf1 ffaf1 changed the title #8975 and #8981 rebased #8975, #8981, #8983 rebased Jun 20, 2023
@ulysses4ever ulysses4ever added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 21, 2023
@ulysses4ever
Copy link
Collaborator

I'm putting the delay-passed passed label because the actual changes have been reviewed in the past and this PR is only a tool to make sure those change land on master safely and sanely. The only remaining issue here is the flaky test. Will see if the change in #9055 helps it, as Gershom proposes.

This helps disabling individual packages (e.g. cabal-testsuite)
This is affect of using indentOfAtLeast method:
any indentation greater than current offset is fine.

That behavior is desirable to parsing multiline field contents,
but it is a bit surprising for fields, which we expect to be aligned.

Such insonsistency seems to be always a mistake, and it's easy to fix once
a machine points it out.
As it runs fourmolu only on modified files, it's a lot faster.
For example `parseGenericPackageDescription` validates input it passes to `readFields'`.

That is desirable: we want try to parse non-UTF8 inputs, and report as a warning if they are not UTF8. (There are non-UTF8 .cabal files on Hackage, e.g. with German names).
@ffaf1 ffaf1 added the merge me Tell Mergify Bot to merge label Jun 22, 2023
@mergify mergify bot merged commit 02e0831 into haskell:master Jun 22, 2023
@Mikolaj
Copy link
Member

Mikolaj commented Jun 22, 2023

Well done. Thank you again.

@ffaf1 ffaf1 deleted the phadej-rebased branch June 22, 2023 17:11
@ffaf1 ffaf1 mentioned this pull request Jul 1, 2023
4 tasks
@ffaf1 ffaf1 mentioned this pull request Mar 28, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants