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

Add unasync.unasync_files() API #58

Merged
merged 1 commit into from May 2, 2020
Merged

Conversation

@pquentin
Copy link
Member

@pquentin pquentin commented May 2, 2020

It allows to run unasync on multiple files using multiples rules if needed, without being tied to setuptools.

@sethmlarson Now that you taught me that using Rule.unasync_files directly made sense, I'm not sure if this is such a good idea. Right now, I have a proof-of-concept that works in hip using this API:

import unasync

unasync.unasync_files(
    [
        "test/with_dummyserver/sync/__init__.py",
        "test/with_dummyserver/async/test_poolmanager.py",
    ],
    rules=[
        unasync.Rule(
            "test/with_dummyserver/async",
            "test/with_dummyserver/sync",
            replacements={
                "AsyncPoolManager": "PoolManager",
                "test_all_backends": "test_sync_backend",
            },
        )
    ],
)

Since I have only a single rule, I guess I could use Rule.unasync_file and loop over files. This pull request feels more general, and since I already had the code, I submitted it. Please tell me what you think.

It allows to run unasync on multiple files using multiples rules if
needed, without being tied to setuptools.
@pquentin pquentin requested a review from sethmlarson May 2, 2020
@codecov
Copy link

@codecov codecov bot commented May 2, 2020

Codecov Report

Merging #58 into master will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   97.60%   97.63%   +0.03%     
==========================================
  Files           2        2              
  Lines         125      127       +2     
  Branches       31       31              
==========================================
+ Hits          122      124       +2     
  Misses          1        1              
  Partials        2        2              
Impacted Files Coverage Δ
src/unasync/__init__.py 97.61% <100.00%> (+0.03%) ⬆️
Copy link
Contributor

@sethmlarson sethmlarson left a comment

I actually think this makes more sense than running Rule.unasync_files() because it's symmetrical with the build_cmdclass() interface.

Maybe we can transition the Rule interface to be private with this PR :)

@pquentin
Copy link
Member Author

@pquentin pquentin commented May 2, 2020

Maybe we can transition the Rule interface to be private with this PR :)

Will open another PR for that + switch back to additional_replacements, then we can cut a release.

@pquentin pquentin merged commit 3ee4a8a into python-trio:master May 2, 2020
4 checks passed
4 checks passed
Travis CI - Pull Request Build Passed
Details
codecov/patch 100.00% of diff hit (target 97.60%)
Details
codecov/project 97.63% (+0.03%) compared to cfd7c7f
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@pquentin pquentin deleted the pquentin:unasync-files branch May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.