> On Dec. 5, 2016, 2:02 a.m., Kevin Klues wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, line 248 > > <https://reviews.apache.org/r/54293/diff/2/?file=1576040#file1576040line248> > > > > Can you add a comment here about the difference between the two > > hashsets? Reading top to bottom, it's not clear exactly what each one > > represents just by looking at the name of the variable. > > Kevin Klues wrote: > Maybe it's just enough to rename the second one to `childFds`.
I added a comment to explain this. > On Dec. 5, 2016, 2:02 a.m., Kevin Klues wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, line 279 > > <https://reviews.apache.org/r/54293/diff/2/?file=1576040#file1576040line279> > > > > Does it make sense to eventually move this function into stout? yeah, I'll add a TODO. We should add a stout/posix/tty.hpp. > On Dec. 5, 2016, 2:02 a.m., Kevin Klues wrote: > > src/slave/containerizer/mesos/io/switchboard.hpp, line 126 > > <https://reviews.apache.org/r/54293/diff/2/?file=1576039#file1576039line126> > > > > It's somewhat arbitrary, but can we make this the second to last > > parameter, in order to group the booleans together? If so, can we do it > > throughout? Let me do that in a separate patch. > On Dec. 5, 2016, 2:02 a.m., Kevin Klues wrote: > > src/slave/containerizer/mesos/io/switchboard.cpp, lines 392-397 > > <https://reviews.apache.org/r/54293/diff/2/?file=1576040#file1576040line392> > > > > I guess you don't need to set the other flags anymore because you rely > > on their default values now? Default is not STDOUT_FILENO and STDERR_FILENO. I'll add a comment here. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54293/#review157937 ----------------------------------------------------------- On Dec. 4, 2016, 9:21 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54293/ > ----------------------------------------------------------- > > (Updated Dec. 4, 2016, 9:21 p.m.) > > > Review request for mesos, Benjamin Hindman and Kevin Klues. > > > Bugs: MESOS-6470 > https://issues.apache.org/jira/browse/MESOS-6470 > > > Repository: mesos > > > Description > ------- > > Supported TTY in I/O switchboard. > > > Diffs > ----- > > include/mesos/slave/containerizer.proto > 33b4c231cc88a048f3de8ab4b2e0e9eb2140132d > src/slave/containerizer/mesos/containerizer.cpp > a7e2665d4c64b7b322d4ae8d5441e8e777236c59 > src/slave/containerizer/mesos/io/switchboard.hpp > 23c66bb2b05310a12fec24a5e5eebddd370ba0f0 > src/slave/containerizer/mesos/io/switchboard.cpp > 778367a268ec350ed438bc9fe9d359d63bdb5503 > src/slave/containerizer/mesos/io/switchboard_main.cpp > 5800217d3562a53218d966c57a14e8b768643c34 > src/slave/containerizer/mesos/launch.cpp > d78ca4df694712e60a15f1795878653eb5e0414e > src/tests/containerizer/io_switchboard_tests.cpp > d82f22b5c9a8d177577db3aeaae9e571e7e2fb80 > > Diff: https://reviews.apache.org/r/54293/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
