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

Do not wrap implicitly concatenated strings used as func args in parens. #3640

Merged
merged 1 commit into from
Apr 28, 2023

Conversation

yilei
Copy link
Contributor

@yilei yilei commented Apr 7, 2023

Description

This reverts the change in #3307, as the changes that would have been introduced in the 2024 style are too destructive based on feedback. See #2188 (comment).

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

diff-shades results comparing this PR (968bb6c) to main (a552f70). The full diff is available in the logs under the "Generate HTML diff report" step.

╭───────────────────────────── Summary ──────────────────────────────╮
│ 16 projects & 517 files changed / 40 051 changes [+15 966/-24 085] │
│                                                                    │
│ ... out of 2 425 728 lines, 11 516 files & 23 projects             │
╰────────────────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

@yilei
Copy link
Contributor Author

yilei commented Apr 28, 2023

@JelleZijlstra any opinion on this? I think the preview string processing overall is a very big change, so I think reverting #3307 would be beneficial to get it in the 2024 stable style.

Copy link
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

OK, let's change this back for now.

@JelleZijlstra JelleZijlstra merged commit e712e48 into psf:main Apr 28, 2023
@yilei yilei deleted the stringwrapping branch June 15, 2023 18:19
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 21, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 21, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 22, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 23, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
PiRK added a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 24, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 26, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
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.

2 participants