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

Black's Changelog Entry and Lint checks that are run on PRs can contradict each other #2070

Closed
MarkCBell opened this issue Mar 27, 2021 · 2 comments · Fixed by #2072
Closed
Labels
T: bug Something isn't working

Comments

@MarkCBell
Copy link
Contributor

MarkCBell commented Mar 27, 2021

Describe the bug

The Changelog Entry Check which is run against PR's requires that CHANGES.md contains a reference the the PR.
However the Lint check runs prettier against CHANGES.md which can sometimes insist on breaking apart a PR reference.

To Reproduce

  1. Add the following line to CHANGES.md:
  - `Black` now requires one-line docstrings to not have leading or trailing spaces (PR #1234)
  1. Submit a PR containing this change.
  2. The Lint / build (3.7) (pull_request) check will fail since this line is too long. It states that this needs to be changed to:
- `Black` now requires one-line docstrings to not have leading or trailing spaces (PR
  #1234)

However making this change results in the Changelog Entry Check (pull_request) failing since it's grep for "PR #1234" does not match the PR reference now that has been split over two lines.

Expected behavior

Changelog Entry Check should match for "PR #1234" even if it is split over multiple lines. So the formatting required by Black can be accepted.

Environment (please complete the following information):

  • Version: master
  • OS and Python version: Github action

Does this bug also happen on master?

Yes

@MarkCBell MarkCBell added the T: bug Something isn't working label Mar 27, 2021
@MarkCBell MarkCBell changed the title Black's Changelog Entry Check and Lint check run on PRs can contradict each other Black's Changelog Entry and Lint checks that are run on PRs can contradict each other Mar 27, 2021
@JelleZijlstra
Copy link
Collaborator

As a workaround, can you put the (PR part on the next line too?

We should ideally make the regex smarter so it accepts this though.

@MarkCBell
Copy link
Contributor Author

MarkCBell commented Mar 27, 2021

As a workaround, can you put the (PR part on the next line too?

No. prettier will reject:

- `Black` now requires one-line docstrings to not have leading or trailing spaces
  (PR #1234)

and insist that this is also changed to:

- `Black` now requires one-line docstrings to not have leading or trailing spaces (PR
  #1234)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants