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 23: Breaking down coala #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

yukiisbored
Copy link
Member

Closes #138

cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
@jayvdb jayvdb requested review from alphadose and manankalra May 10, 2018 14:20
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
cEP-0023.md Outdated Show resolved Hide resolved
@xferra
Copy link
Member

xferra commented May 11, 2018

Seems like that questions from #117 are relevant for this proposal as well.

@jayvdb
Copy link
Member

jayvdb commented Jun 17, 2018

Once coala/coala-bears#2446 is merged, we can experiment with generating Bear source from yaml files, which will help establish feasibility.
It will be interesting to see how many of the current bears are able to be expressed as a basic yaml dataset.

There is also the potential for generating test modules, and possibly those test modules will be better than the current ones as they will benefit from generics.

@yukiisbored yukiisbored requested a review from li-boxuan as a code owner January 1, 2019 06:35
@yukiisbored yukiisbored changed the title cEP 23: Separation of bears' metadata cEP 23: Breaking down coala Jan 1, 2019
# Breaking down coala

| Metadata | |
|----------|-----------------------------------------------|

Choose a reason for hiding this comment

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

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -1,7 +1,7 @@
 # Breaking down coala
 
 | Metadata |                                               |
-|----------|-----------------------------------------------|
+| -------- | --------------------------------------------- |
 | cEP      | 0023                                          |
 | Version  | 1.0                                           |
 | Title    | Breaking down the coala                       |

local = true
executable = space_consistency_bear.py
output_regex = .+:(?P<line>\d+): (?P<message>.*)

Choose a reason for hiding this comment

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

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -151,7 +151,6 @@
 local = true
 executable = space_consistency_bear.py
 output_regex = .+:(?P<line>\d+): (?P<message>.*)
-

The coala Engine will intepret the following file, setup the command arguments,

## The RPC

The RPC will follow the JSON-RPC specification
(https://www.jsonrpc.org/specification).

Choose a reason for hiding this comment

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

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -171,7 +171,7 @@
 ## The RPC
 
 The RPC will follow the JSON-RPC specification
-(https://www.jsonrpc.org/specification).
+(<https://www.jsonrpc.org/specification>).
 
 The methods will be the following:
   * `start`

(https://www.jsonrpc.org/specification).

The methods will be the following:
* `start`

Choose a reason for hiding this comment

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

The text does not comply to the set style.

Origin: MarkdownBear, Section: markdown.

The issue can be fixed by applying the following patch:

--- a/tmp/tmpzr0a2jlu/cEP-0023.md
+++ b/tmp/tmpzr0a2jlu/cEP-0023.md
@@ -174,10 +174,12 @@
 (https://www.jsonrpc.org/specification).
 
 The methods will be the following:
-  * `start`
-  * `commit`
+
+- `start`
+- `commit`
 
 The following examples will use the syntax below:
+

--> data sent to the Engine
<-- data sent to the Frontend


### `commit`

`commit` will commit an action proposed by the Engine and pass onto the next warning.

Choose a reason for hiding this comment

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

Line must be at most 80 characters maximum-line-length remark-lint

Origin: MarkdownBear, Section: markdown.

addition, bears in essence can do anything it wants to since it is just a Python
class that is loaded into the coala process.

This caused numerous unwanted regressions between bears and coala and costed a
Copy link
Member

Choose a reason for hiding this comment

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

Maybe an example here? What regression/problem did the current architecture bring about?


Meanwhile, the coala Engine's sole job is to intepret the bear files and
execute/supervise the linter/helper programs along with committing the action
that is requsted by the user.
Copy link
Member

Choose a reason for hiding this comment

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

a typo here :P

Bears are now just simple formatted plain-text file containing the metadata
needed. It can also contain a simple program as helpers or sole linters.

coala will separated into two parts: The coala Engine and The coala CLI.
Copy link
Member

Choose a reason for hiding this comment

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

be separated

the coala Engine. The CLI will present the output of the Engine and present them
with possible actions that the user can do.

Meanwhile, the coala Engine's sole job is to intepret the bear files and
Copy link
Member

Choose a reason for hiding this comment

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

a typo: intepret

before the program (e.g generating the configuration file) and do stuff
after it (e.g cleaning).

The frontend and backend programs will communicate to each other via json-rpc.
Copy link
Member

Choose a reason for hiding this comment

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

It is not very clear which programs frontend and backend are referring to. I assume frontend = coala CLI and backend = coala Engine, but it would be better to be more precise.

Bears are now just simple formatted plain-text file containing the metadata
needed. It can also contain a simple program as helpers or sole linters.

coala will separated into two parts: The coala Engine and The coala CLI.
Copy link
Member

Choose a reason for hiding this comment

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

Instead of one coala process, there will be two separate processes in the new architecture, is my understanding correct? Maybe this paragraph can be elaborated a bit more :)

after it (e.g cleaning).

The frontend and backend programs will communicate to each other via json-rpc.
json-rpc is chosen for it's simplicity.
Copy link
Member

Choose a reason for hiding this comment

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

One advantage of using RPC I can think of is: in the future the coala Engine might be able to run on the cloud and users only need a coala CLI running locally.

But do we need an RPC framework if we only use it for local interprocess communication?

Copy link
Member

Choose a reason for hiding this comment

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

I think we can use JSON-RPC because it is easier than other IPC/RPC implementations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we want it to be easily used by other programs such as plugins for a text editor or maybe a GUI version of the frontend.

@li-boxuan
Copy link
Member

Seems this proposal involves two parts:

part (1). separate coala core into 2 parts.
part (2). use a new way to write bears.

Are they independent? Is breaking down coala a necessary step for part (2)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants