Skip to content
This repository has been archived by the owner on Apr 22, 2020. It is now read-only.

Close #13 - better lockstep API #31

Merged
merged 1 commit into from
Aug 28, 2017
Merged

Close #13 - better lockstep API #31

merged 1 commit into from
Aug 28, 2017

Conversation

idanarye
Copy link

Assuming:

class Foo:
    @lockstep
    def process(self):
        # ...

foo = Foo()
  • foo.process() - run the entire process
  • with foo.process.lockstep() as process: - step into the process
  • yield from foo.process.lockstep() - for lockstep nesting

@idanarye idanarye requested a review from koreno June 13, 2017 14:42
@idanarye
Copy link
Author

Not 100% sure about the yield from API. Maybe now that we are explicitly running the lockstep, we should also give the yield from it's own API? Something like yield from foo.process.delegate()?

@idanarye
Copy link
Author

BTW: please let me merge this. I want to merge it and immediately update testlight - otherwise it'll break the installation.

@koreno
Copy link
Contributor

koreno commented Jun 13, 2017

Shouldn't it be

with foo.process.lockstep() as process:
    yield from process

?

@idanarye
Copy link
Author

Currently we allow yielding from the context manager itself.

Yielding from the process object is easy to implement, but I generally prefer APIs that allow their user to avoid binding new names.

I would like to have a separate method for yielding the steps, because having it under .lockstep() forces me to use _LockstepContextManagerWrapper instead of a regular contextmanager, but I struggle to find a good name for it:

  • yield from foo.process.unwrap_lockstep()?
  • yield from foo.process.nested_lockstep()?
  • yield from foo.process.lockstep_steps()?

@idanarye
Copy link
Author

?

Assuming:
```python
class Foo:
    @lockstep
    def process(self):
        # ...

foo = Foo()
```

* `foo.process()` - run the entire process
* `with foo.process.lockstep() as process:` - step into the process
* `with foo.process.lockstep() as process: yield from process` - for lockstep nesting
@idanarye idanarye merged commit 2ba437d into master Aug 28, 2017
@YuvalEvron YuvalEvron deleted the better-lockstep-API branch August 6, 2019 08:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants