beerriot
commented
11 years ago Thanks for the improvements, @macintux. I'd like to get the two minor bits I commented on fixed up, but then I'll get it merged.
Closed macintux closed 11 years ago
beerriot
commented
11 years ago Thanks for the improvements, @macintux. I'd like to get the two minor bits I commented on fixed up, but then I'll get it merged.
macintux
commented
11 years ago Curious, I would have sworn the underscores didn't show up in my branch. Must have been looking at my master branch; apologies. Frustrating that we can't have org-mode output and Github rendering at the same time, unless there's some other way to split the baby.
Introducing the term coordinator definitely concerned me with regard to implications for naming and comments in the code, so I'm glad to hear you don't think it'll be a major obstacle. I'll take a look soonish.
beerriot
commented
11 years ago I think a good compromise might be to use the inline literal mode in those headings, by surrounding with equals signs: =riak_pipe_w_tee=, for example. That should render right-ish in both places. If it doesn't, github rendering should take precedence.
Yes, my only concern with the "coordinator" change is that without some direct links between README.org and riak_pipe_fitting.erl, it might not be obvious how to match up doc and source. "Coordinator" might not be the perfect term for the concept, but as you described, it's better than the confusion with the overused "fitting".
macintux
commented
11 years ago Fixed headings in README.org. Updated comments in src/riak_pipe_fitting.erl.
Also tweaked comments in src/riak_pipe_fitting_sup.erl and src/riak_pipe_w_crash.erl.
Ready for review.
beerriot
commented
11 years ago +1; will merge to master as soon as jared finishes rc tagging
Hodgepodge of small and major changes. Changed terminology to what I feel is much more explicit and easier to understand, so please review carefully.
When reading through the document carefully, I really struggled with the distinction between the workers and the fittings processes. In hindsight it's obvious, but it wasn't on first (or second) reading through the first few sections.
Also, in a couple of places the README file failed to include "process" when talking about fittings processes, so there was some unnecessary ambiguity between the concept, the metadata, and the processes themselves.
Rather than just tweak those references, I decided to rename "fittings process" to "coordinator". I feel this makes the role much more explicit, plus we use "process" so often (verb and noun) I think it helps with the flow. Note that I also shortened "worker process" to "worker" in the very few places where the former occurred.
I also introduced "sink" earlier, because it seemed like a natural fit while introducing other key actors.
Finally, there were various minor glitches to fix, such as a broken link and section titles with underscores that needed to be escaped for HTML generation to work properly.