Improve drcachesim -indir and -outdir docs: directory contents are internal #6786
Conversation
…ories If the user happens to, for example, create a file in this directory then drcachesim will complain that it doesn't understand the file. Warn the user that the contents of these directories are internal to the tool.
| @@ -72,7 +72,8 @@ droption_t<std::string> op_ipc_name( | |||
| droption_t<std::string> op_outdir( | |||
There was a problem hiding this comment.
Improve -indir,-outdir docs, directory contents are internal
grammar: a comma cannot connect separate sentences: replace with a colon or similar
There was a problem hiding this comment.
That it's technically bad grammer is left as a given.
That this level of grammar correctness is required is new ... noted for future reference. :-)
There was a problem hiding this comment.
The comma is still there.
There was a problem hiding this comment.
Which comma? The comma in "Improve drcachesim -indir and -outdir docs, directory contents are internal" ?
There was a problem hiding this comment.
Yes, see the original review request. Oh I see now that there was another comma which maybe caused confusion though asking for the colon should disambiguate: my original request is s/docs,/docs:/.
xdje42
changed the title
|
Ping |
Normally you'd click on the re-request review to indicate it's ready for a look: otherwise the reviewer will assume it's still being worked on. |
|
| @@ -1146,6 +1146,8 @@ $ bin64/drrun -t drcachesim -indir drmemtrace.app.pid.xxxx.dir/ | |||
|
|
|||
| The direct results of the \p -offline run are raw, compacted files, stored | |||
| in a \p raw/ subdirectory of the \p drmemtrace.app.pid.xxxx.dir directory. | |||
| The contents of this directory are internal to the tool. Do not alter, add, | |||
| or delete files here. | |||
| The \p -indir option both converts the data to a canonical trace form and | |||
| passes the resulting data to the cache simulator. The canonical trace data | |||
| is stored by \p -indir in a \p trace/ subdirectory inside the \p | |||
There was a problem hiding this comment.
The same warning applies to the trace/ subdir too. (Or clarify the above to explicitly state it's talking about the outer dir and not the raw/ subdir.)
There was a problem hiding this comment.
How about say that the contents of drmemtrace.app.pid.xxxx.dir, and all its subdirectories, are internal to the tool ?
xdje42
changed the title
| @@ -1147,6 +1147,8 @@ $ bin64/drrun -t drmemtrace -indir drmemtrace.app.pid.xxxx.dir/ | |||
|
|
|||
| The direct results of the \p -offline run are raw, compacted files, stored | |||
| in a \p raw/ subdirectory of the \p drmemtrace.app.pid.xxxx.dir directory. | |||
| The contents of \p drmemtrace.app.pid.xxxx.dir, and all its subdirectories, | |||
There was a problem hiding this comment.
nit: missing "the": s/of/of the/
(be consistent with all the other references such as line 1149 and line 1154 which have "the")
There was a problem hiding this comment.
I'd expect the sole addition of "the" here to be bad grammar.
The other instances you point out say "... of the drmemtrace.app.pid.xxxx.dir directory",
whereas here the wording is "... of drmemtrace.app.pid.xxxx.dir".
Shall I add "directory" as well ? [gotta get the grammar correct! :-)]
i.e., "The contents of the \p drmemtrace.app.pid.xxx.dir directory, and all its subdirectories, ..."
There was a problem hiding this comment.
Ah agreed, must have read it as the same as the others: good as-is or with "the ... directory".
| @@ -72,7 +72,8 @@ droption_t<std::string> op_ipc_name( | |||
| droption_t<std::string> op_outdir( | |||
There was a problem hiding this comment.
Given the rebranding of the framework as "drmemtrace" (because "drcachesim" is just one tool inside the framework), best to s/drcachesim/drmemtrace/ in the PR title and description.
There was a problem hiding this comment.
Except that these docs are specifically for drcachesim?
There was a problem hiding this comment.
No, these docs are not for the drcachesim simulator: they are for the drmemtrace framework's tracer and post-processor and front-end for running a variety of tools (just one of which is drcachesim).
If the user happens to, for example, create a file in this directory then drcachesim will complain that it doesn't understand the file. Warn the user that the contents of these directories are internal to the tool.