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

cEP-0030: Next Generation Action System #182

Merged
merged 2 commits into from
Jun 6, 2021

Conversation

akshatkarani
Copy link
Member

Closes #181

@akshatkarani akshatkarani force-pushed the cEP-0030 branch 2 times, most recently from d39d989 to 1095d09 Compare May 11, 2019 15:26
@akshatkarani
Copy link
Member Author

@abhishalya
Copy link
Member

  1. Max 80 chars per line please.
  2. I expect more implementation details, these seem too vague to me rn.

Ping @Makman2

@Makman2
Copy link
Member

Makman2 commented May 14, 2019

I would also expect a few more details. Especially basic implementation approach and how to integrate it with current coala's internas.

cEP-0030.md Outdated
```python

ACTIONS = [DoNothingAction(),
ApplyPatchAction(),
Copy link
Member

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?

Copy link
Member Author

@akshatkarani akshatkarani Jun 25, 2019

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.

Copy link
Member Author

@akshatkarani akshatkarani Jun 25, 2019

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.

cEP-0030.md Outdated Show resolved Hide resolved
@akshatkarani
Copy link
Member Author

@Makman2 Updated cEP. You can check out the PRs here coala/coala#6029 coala/coala-bears#2927

@akshatkarani akshatkarani force-pushed the cEP-0030 branch 2 times, most recently from 06b2afe to 0e76d9e Compare June 26, 2019 11:31
@akshatkarani akshatkarani force-pushed the cEP-0030 branch 2 times, most recently from a914ce9 to 42015fc Compare July 17, 2019 15:59
Copy link
Member

@abhishalya abhishalya left a 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

cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
cEP-0030.md Outdated Show resolved Hide resolved
abhishalya
abhishalya previously approved these changes Aug 11, 2019
@abhishalya abhishalya merged commit b8a2d2b into coala:master Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

cEP-0030: Next Generation Action System
3 participants