-
Notifications
You must be signed in to change notification settings - Fork 53
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
cEP-0030: Next Generation Action System #182
Conversation
d39d989
to
1095d09
Compare
Ping @Makman2 |
I would also expect a few more details. Especially basic implementation approach and how to integrate it with current coala's internas. |
cb44845
to
1b18c5e
Compare
cEP-0030.md
Outdated
```python | ||
|
||
ACTIONS = [DoNothingAction(), | ||
ApplyPatchAction(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this meaningless without supplying a specific patch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not changing the current actions and adding a init
method to it. Instead result.actions
will only have actions that bears have defined. See the PR coala/coala#6029 for the implementation of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyways a test was failing because of this change and I removed it from PR. So I will update here as well.
@Makman2 Updated cEP. You can check out the PRs here coala/coala#6029 coala/coala-bears#2927 |
06b2afe
to
0e76d9e
Compare
a914ce9
to
42015fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks okay to me.
Ping @Makman2
Closes #181