Skip to content

Add architecture documentation#125

Merged
gaocegege merged 4 commits intomasterfrom
docs/tech-report
Jun 20, 2017
Merged

Add architecture documentation#125
gaocegege merged 4 commits intomasterfrom
docs/tech-report

Conversation

@gaocegege
Copy link
Copy Markdown
Member

Close #119

Signed-off-by: Ce Gao ce.gao@outlook.com

Signed-off-by: Ce Gao <ce.gao@outlook.com>
Signed-off-by: Ce Gao <ce.gao@outlook.com>
@gaocegege
Copy link
Copy Markdown
Member Author

gaocegege commented Jun 18, 2017

PTAL @jeremydouglass @hawkingrei

I am not confident in my English 🤔

@gaocegege gaocegege changed the title WIP: Add architecture documentation Add architecture documentation Jun 18, 2017
Copy link
Copy Markdown
Member

@jeremydouglass jeremydouglass left a comment

Choose a reason for hiding this comment

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

This is a very good draft! The important details are here, it is well organized, and the English is clear. My main feedback:

  1. Order of the overview: Say what this project is immediately, then give background on what Processing is. Currently the project is introduced on the fourth paragraph, this is confusing.
  2. There are a few typos, like "skectches." Run spellcheck before submitting.
  3. Some of the English could be slightly improved.

Rather than give details, I will add a commit.

...plus, I think architecture needs to be re-organized to be more clear. Adding an additional commit -- please read and see if this is an improvement.

Reorganize introduction, small changes.
Reorganize Architecture section, group in logical order by common/jar/pde, expand descriptions.
Comment thread raw-docs/architecture.md

#### `ModeService` and `SketchServiceManager`

`ModeService` is a Remote Method Invocation(RMI) interface, which extends `java.rmi.Remote`. `SketchServiceManager` is the class which implements `ModeService`. `RLangMode` creates a `SketchServiceManager` instance when started. `SketchServiceManager` manages all `SketchServiceRunner`s, and `SketchServiceRunner` runs the real logic of Processing.R sketches.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

s/a/an?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not sure I understand. Is this sed for changing to "an Remote Method Invocation" and "an SketchServiceManager" ? Those changes aren't needed.

"a" comes before closed consonant letters and sounds here like "Remote" and "Sketch", "an" would come before open vowel letters and sounds like "Interface" or "Extends."

Copy link
Copy Markdown
Member Author

@gaocegege gaocegege Jun 20, 2017

Choose a reason for hiding this comment

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

OK, I got it! I thought we need to change from a Remote to an Remote.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In spoken English, people would say "a Remote" (AY reemOHT). However, they would say "an R.M.I." (AAN AHR-EHM-IY). Changing "a" to "an" is a way to keep the two vowel sounds (AY AHR) from blending together.

@gaocegege
Copy link
Copy Markdown
Member Author

LGTM 🎉

@gaocegege gaocegege merged commit e833236 into master Jun 20, 2017
@gaocegege gaocegege deleted the docs/tech-report branch June 20, 2017 06:29
@gaocegege gaocegege modified the milestone: Evaluation 1 Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants