mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benno Evers <bev...@mesosphere.com>
Subject Re: Review Request 63372: Added documentation for memory profiling.
Date Tue, 24 Apr 2018 14:42:05 GMT


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 31 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line31>
> >
> >     the bundled version?

I think 'a' is correct here.


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 82-84 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line82>
> >
> >     Please tell people they will get an `id` of the progiling run that can be used
to uniqly identify it.

I think its better not to overcomplicate: For people trying things for the first time and
following the guide, concurrent runs will be a non-issue, and it's mentioned later in the
scripting section anyways.


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 98 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line98>
> >
> >     comma after i.e.

Not according to the internet: https://english.stackexchange.com/questions/16172/should-i-always-use-a-comma-after-e-g-or-i-e


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 122-124 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line122>
> >
> >     Is `dot` tool required as well?

Wasn't that what you found testing on Mac?


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 168 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line168>
> >
> >     Does \~\~\~{.bash} work?

It works in the sense that it produces a monospace code block (i.e. no syntax highlighting),
but the style guide says this so I assume we shouldn't use it:

```
**NOTE:** Because of shortcomings of Doxygen's markdown parser we currently use indentation
for wrapping all non C++ code blocks.
```


> On April 23, 2018, 10:01 p.m., Alexander Rukletsov wrote:
> > docs/memory-profiling.md
> > Lines 192 (patched)
> > <https://reviews.apache.org/r/63372/diff/7/?file=2004744#file2004744line192>
> >
> >     Let's have an inline link in form []() for consistency.

The problem is that the url itself contains a `)`, confusing the markdown parser. It's possible
to escape that `)` with a backslash, but then the link will be incorrect for people reading
the markdown directly.


- Benno


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63372/#review201770
-----------------------------------------------------------


On April 24, 2018, 2:40 p.m., Benno Evers wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63372/
> -----------------------------------------------------------
> 
> (Updated April 24, 2018, 2:40 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added documentation for memory profiling.
> 
> 
> Diffs
> -----
> 
>   CHANGELOG c9d5c8bb92ba70982ceb4c90894172f8523e8e3b 
>   docs/home.md 91d5bcbd4befdd801a10929cd71d1110c91bc02f 
>   docs/memory-profiling.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63372/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Benno Evers
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message