Skip to content

Add cpython Lib/ repository config into primer config - Disabled#2429

Merged
ichard26 merged 5 commits into
mainfrom
cpython_primer
Aug 24, 2021
Merged

Add cpython Lib/ repository config into primer config - Disabled#2429
ichard26 merged 5 commits into
mainfrom
cpython_primer

Conversation

@cooperlees
Copy link
Copy Markdown
Collaborator

  • cpython tests is probably the best repo for black to test on as the stdlib's unittests should use all syntax
    • Limit to running in recent versions of the python runtime - e.g. today >= 3.9
      • This allows us to parse more syntax
  • Exclude all failing files for now
  • Add new black command arguments logging in debug mode; very handy for seeing how CLI arguments are formatted

cython now succeeds ignoring 16 files:

Oh no! 💥 💔 💥
1859 files would be reformatted, 148 files would be left unchanged.

Testing

  • Ran locally with and without string processing - Very little runtime difference BUT 3 more failed files
time /tmp/tb/bin/black --experimental-string-processing --check . 2>&1 | tee /tmp/black_cpython_esp
...
Oh no! 💥 💔 💥
1859 files would be reformatted, 148 files would be left unchanged, 16 files would fail to reformat.

real	4m8.563s
user	16m21.735s
sys	0m6.000s
  • Add unittest for new covienence config file flattening that allows long arguments to be broke up into an array/list of strings

Addresses #2407

- cpython tests is probably the best repo for black to test on as the stdlib's unittests should use all syntax
  - Limit to running in recent versions of the python runtime - e.g. today >= 3.9
    - This allows us to parse more syntax
- Exclude all failing files for now
  - Definately have bugs to explore there - Refer to #2407 for more details there
  - Some test files on purpose have syntax errors, so we will never be able to parse them
- Add new black command arguments logging in debug mode; very handy for seeing how CLI arguments are formatted

cython now succeeds ignoring 16 files:
```
Oh no! 💥 💔 💥
1859 files would be reformatted, 148 files would be left unchanged.
```

Testing
- Ran locally with and without string processing - Very little runtime difference BUT 3 more failed files
```
time /tmp/tb/bin/black --experimental-string-processing --check . 2>&1 | tee /tmp/black_cpython_esp
...
Oh no! 💥 💔 💥
1859 files would be reformatted, 148 files would be left unchanged, 16 files would fail to reformat.

real	4m8.563s
user	16m21.735s
sys	0m6.000s
```
- Add unittest for new covienence config file flattening that allows long arguments to be broke up into an array/list of strings

Addresses #2407
@cooperlees cooperlees added the C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases label Aug 15, 2021
- If present, will set forked process limit to that value in seconds
- Otherwise, stay with default 10 minutes (600 seconds)
@cooperlees cooperlees added the ci: skip news Pull requests that don't need a changelog entry. label Aug 15, 2021
- Rather than start at the repo root start at a configured path within the repository
  - e.g. for cpython only run black on `Lib`
@cooperlees cooperlees changed the title Add cpython repository into primer runs Add cpython Lib/ repository into primer runs Aug 16, 2021
@cooperlees
Copy link
Copy Markdown
Collaborator Author

Not going to work. But since I did all the work diagnosing all the files we break on, I'd love to leave the config so interested people could use this to fix the bugs it exposes ... But can give up if people don't want the noise there.

@cooperlees cooperlees changed the title Add cpython Lib/ repository into primer runs Add cpython Lib/ repository config into primer config - Disabled Aug 16, 2021
@cooperlees
Copy link
Copy Markdown
Collaborator Author

So, should I close this or do we want to merge for people to use in the future when people have cycles for potentially looking into the bugs this finds?

@ichard26
Copy link
Copy Markdown
Collaborator

So, should I close this or do we want to merge for people to use in the future when people have cycles for potentially looking into the bugs this finds?

I don't know how likely it is people will look into the discovered issues, but this can't possibly hurt and would be useful so I'm merging.

Comment thread src/black_primer/lib.py Outdated
Copy link
Copy Markdown
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

Hopefully my edit via the web interface doesn't break the tests. Will merge afterwards.

Comment thread src/black_primer/lib.py
Comment on lines +136 to +137
args_as_str = "".join(arg)
flat_args.append(args_as_str)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Nice - Much more pythonic

@ichard26 ichard26 merged commit 5bb4da0 into main Aug 24, 2021
@ichard26 ichard26 deleted the cpython_primer branch August 24, 2021 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C: maintenance Related to project maintenance, e.g. CI, testing, policy changes, releases ci: skip news Pull requests that don't need a changelog entry.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants