| Message ID | 20230829143541.3837919-1-jose.quaresma@foundries.io |
|---|---|
| State | New |
| Headers | show |
| Series | pybootchartgui: also match subtasks of the main ones | expand |
On Tue, 2023-08-29 at 14:35 +0000, Jose Quaresma wrote: > This will match other deviation subtask of the same main task, > a couple of them can be found on oe-core layer: > do_compile_kernelmodules > do_compile_ptest > native_add_do_populate_sysroot_deps > do_package_qa > cmake_do_configure > setuptools3_do_configure > cargo_common_do_configure > python_pyo3_do_configure > python_setuptools3_rust_do_configure > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > --- > scripts/pybootchartgui/pybootchartgui/draw.py | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/scripts/pybootchartgui/pybootchartgui/draw.py b/scripts/pybootchartgui/pybootchartgui/draw.py > index 3d1ff695c1..2beb3c7c67 100644 > --- a/scripts/pybootchartgui/pybootchartgui/draw.py > +++ b/scripts/pybootchartgui/pybootchartgui/draw.py > @@ -661,20 +661,20 @@ def render_processes_chart(ctx, options, trace, curr_y, width, h, sec_w): > > #print("proc at %s %s %s %s" % (x, y, w, proc_h)) > col = None > - if task == "do_compile": > + if "_setscene" in task: > + col = WHITE > + elif "do_compile" in task: > col = TASK_COLOR_COMPILE > - elif task == "do_configure": > + elif "do_configure" in task: > col = TASK_COLOR_CONFIGURE > - elif task == "do_install": > + elif "do_install" in task: > col = TASK_COLOR_INSTALL > - elif task == "do_populate_sysroot": > + elif "do_populate_sysroot" in task: > col = TASK_COLOR_SYSROOT > - elif task == "do_package": > - col = TASK_COLOR_PACKAGE > - elif task == "do_package_write_rpm" or \ > - task == "do_package_write_deb" or \ > - task == "do_package_write_ipk": > + elif "do_package_write" in task: > col = TASK_COLOR_PACKAGE_WRITE > + elif "do_package" in task: > + col = TASK_COLOR_PACKAGE > else: > col = WHITE > I'm a bit torn on this as the patch changes the approach of the code. Currently, the tasks are marked without any fuzz, i.e. it matches do_compile alone. The code is entirely consistent in that all the other areas match specific tasks too. After the change you're changing the meaning to "any compile task" and "any configure task". I'm not entirely convinced this is a good thing. Should do_package match do_package_qa for example? Those are quite different. I think we're swapping something which is currently at least quite clear for something which is very fuzzy and not easy for the user to understand without looking at the code :/. Cheers, Richard
Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia quinta, 7/09/2023 à(s) 15:16: > On Tue, 2023-08-29 at 14:35 +0000, Jose Quaresma wrote: > > This will match other deviation subtask of the same main task, > > a couple of them can be found on oe-core layer: > > do_compile_kernelmodules > > do_compile_ptest > > native_add_do_populate_sysroot_deps > > do_package_qa > > cmake_do_configure > > setuptools3_do_configure > > cargo_common_do_configure > > python_pyo3_do_configure > > python_setuptools3_rust_do_configure > > > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > > --- > > scripts/pybootchartgui/pybootchartgui/draw.py | 18 +++++++++--------- > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > diff --git a/scripts/pybootchartgui/pybootchartgui/draw.py > b/scripts/pybootchartgui/pybootchartgui/draw.py > > index 3d1ff695c1..2beb3c7c67 100644 > > --- a/scripts/pybootchartgui/pybootchartgui/draw.py > > +++ b/scripts/pybootchartgui/pybootchartgui/draw.py > > @@ -661,20 +661,20 @@ def render_processes_chart(ctx, options, trace, > curr_y, width, h, sec_w): > > > > #print("proc at %s %s %s %s" % (x, y, w, proc_h)) > > col = None > > - if task == "do_compile": > > + if "_setscene" in task: > > + col = WHITE > > + elif "do_compile" in task: > > col = TASK_COLOR_COMPILE > > - elif task == "do_configure": > > + elif "do_configure" in task: > > col = TASK_COLOR_CONFIGURE > > - elif task == "do_install": > > + elif "do_install" in task: > > col = TASK_COLOR_INSTALL > > - elif task == "do_populate_sysroot": > > + elif "do_populate_sysroot" in task: > > col = TASK_COLOR_SYSROOT > > - elif task == "do_package": > > - col = TASK_COLOR_PACKAGE > > - elif task == "do_package_write_rpm" or \ > > - task == "do_package_write_deb" or \ > > - task == "do_package_write_ipk": > > + elif "do_package_write" in task: > > col = TASK_COLOR_PACKAGE_WRITE > > + elif "do_package" in task: > > + col = TASK_COLOR_PACKAGE > > else: > > col = WHITE > > > > I'm a bit torn on this as the patch changes the approach of the code. > > Currently, the tasks are marked without any fuzz, i.e. it matches > do_compile alone. The code is entirely consistent in that all the other > areas match specific tasks too. > > After the change you're changing the meaning to "any compile task" and > "any configure task". > Yeah that was my intention and they still be consistent by color but now matching a sequential group of tasks: "any configure task" -> "any compile task" -> "any package task" -> "any package write task" -> "do_populate_sysroot" and in this way it is easier to identify the color patterns of the above groups, currently we get a couple of white bars for the majority of the tasks. > I'm not entirely convinced this is a good thing. Should do_package > match do_package_qa for example? Those are quite different. I think > we're swapping something which is currently at least quite clear for > something which is very fuzzy and not easy for the user to understand > without looking at the code :/. > I can rework the patch to have the same colors as before with more new colors for the new tasks I added but with many colors It will be harder to understand. Another option can be just group the configure and compile tasks as these usually do the same thing as the ones alone. Anyway feel free to drop this patch since no serious problem is being fixed and it would be more the usability that would be affected. Jose > Cheers, > > Richard > > >
On Thu, 2023-09-07 at 15:59 +0100, Jose Quaresma wrote: > > > Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia > quinta, 7/09/2023 à(s) 15:16: > > On Tue, 2023-08-29 at 14:35 +0000, Jose Quaresma wrote: > > > This will match other deviation subtask of the same main task, > > > a couple of them can be found on oe-core layer: > > > do_compile_kernelmodules > > > do_compile_ptest > > > native_add_do_populate_sysroot_deps > > > do_package_qa > > > cmake_do_configure > > > setuptools3_do_configure > > > cargo_common_do_configure > > > python_pyo3_do_configure > > > python_setuptools3_rust_do_configure > > > > > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > > > --- > > > scripts/pybootchartgui/pybootchartgui/draw.py | 18 +++++++++--- > > > ------ > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > diff --git a/scripts/pybootchartgui/pybootchartgui/draw.py > > > b/scripts/pybootchartgui/pybootchartgui/draw.py > > > index 3d1ff695c1..2beb3c7c67 100644 > > > --- a/scripts/pybootchartgui/pybootchartgui/draw.py > > > +++ b/scripts/pybootchartgui/pybootchartgui/draw.py > > > @@ -661,20 +661,20 @@ def render_processes_chart(ctx, options, > > > trace, curr_y, width, h, sec_w): > > > > > > #print("proc at %s %s %s %s" % (x, y, w, proc_h)) > > > col = None > > > - if task == "do_compile": > > > + if "_setscene" in task: > > > + col = WHITE > > > + elif "do_compile" in task: > > > col = TASK_COLOR_COMPILE > > > - elif task == "do_configure": > > > + elif "do_configure" in task: > > > col = TASK_COLOR_CONFIGURE > > > - elif task == "do_install": > > > + elif "do_install" in task: > > > col = TASK_COLOR_INSTALL > > > - elif task == "do_populate_sysroot": > > > + elif "do_populate_sysroot" in task: > > > col = TASK_COLOR_SYSROOT > > > - elif task == "do_package": > > > - col = TASK_COLOR_PACKAGE > > > - elif task == "do_package_write_rpm" or \ > > > - task == "do_package_write_deb" or \ > > > - task == "do_package_write_ipk": > > > + elif "do_package_write" in task: > > > col = TASK_COLOR_PACKAGE_WRITE > > > + elif "do_package" in task: > > > + col = TASK_COLOR_PACKAGE > > > else: > > > col = WHITE > > > > > > > I'm a bit torn on this as the patch changes the approach of the > > code. > > > > Currently, the tasks are marked without any fuzz, i.e. it matches > > do_compile alone. The code is entirely consistent in that all the > > other > > areas match specific tasks too. > > > > After the change you're changing the meaning to "any compile task" > > and > > "any configure task". > > > > > Yeah that was my intention and they still be consistent by color but > now > matching a sequential group of tasks: > > "any configure task" -> "any compile task" -> "any package task" -> > "any package write task" -> "do_populate_sysroot" > > and in this way it is easier to identify the color patterns of the > above groups, > currently we get a couple of white bars for the majority of the > tasks. > > > > > I'm not entirely convinced this is a good thing. Should do_package > > match do_package_qa for example? Those are quite different. I think > > we're swapping something which is currently at least quite clear > > for > > something which is very fuzzy and not easy for the user to > > understand > > without looking at the code :/. > > > > > I can rework the patch to have the same colors as before with more > new colors > for the new tasks I added but with many colors It will be harder to > understand. How about a compromise. Keep do_compile and "compile like" tasks separate and use a pale version of the compile colour for "compile like". Do something similar for configure and leave the others unchanged? Cheers, Richard
Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia quinta, 7/09/2023 à(s) 16:20: > On Thu, 2023-09-07 at 15:59 +0100, Jose Quaresma wrote: > > > > > > Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia > > quinta, 7/09/2023 à(s) 15:16: > > > On Tue, 2023-08-29 at 14:35 +0000, Jose Quaresma wrote: > > > > This will match other deviation subtask of the same main task, > > > > a couple of them can be found on oe-core layer: > > > > do_compile_kernelmodules > > > > do_compile_ptest > > > > native_add_do_populate_sysroot_deps > > > > do_package_qa > > > > cmake_do_configure > > > > setuptools3_do_configure > > > > cargo_common_do_configure > > > > python_pyo3_do_configure > > > > python_setuptools3_rust_do_configure > > > > > > > > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> > > > > --- > > > > scripts/pybootchartgui/pybootchartgui/draw.py | 18 +++++++++--- > > > > ------ > > > > 1 file changed, 9 insertions(+), 9 deletions(-) > > > > > > > > diff --git a/scripts/pybootchartgui/pybootchartgui/draw.py > > > > b/scripts/pybootchartgui/pybootchartgui/draw.py > > > > index 3d1ff695c1..2beb3c7c67 100644 > > > > --- a/scripts/pybootchartgui/pybootchartgui/draw.py > > > > +++ b/scripts/pybootchartgui/pybootchartgui/draw.py > > > > @@ -661,20 +661,20 @@ def render_processes_chart(ctx, options, > > > > trace, curr_y, width, h, sec_w): > > > > > > > > #print("proc at %s %s %s %s" % (x, y, w, proc_h)) > > > > col = None > > > > - if task == "do_compile": > > > > + if "_setscene" in task: > > > > + col = WHITE > > > > + elif "do_compile" in task: > > > > col = TASK_COLOR_COMPILE > > > > - elif task == "do_configure": > > > > + elif "do_configure" in task: > > > > col = TASK_COLOR_CONFIGURE > > > > - elif task == "do_install": > > > > + elif "do_install" in task: > > > > col = TASK_COLOR_INSTALL > > > > - elif task == "do_populate_sysroot": > > > > + elif "do_populate_sysroot" in task: > > > > col = TASK_COLOR_SYSROOT > > > > - elif task == "do_package": > > > > - col = TASK_COLOR_PACKAGE > > > > - elif task == "do_package_write_rpm" or \ > > > > - task == "do_package_write_deb" or \ > > > > - task == "do_package_write_ipk": > > > > + elif "do_package_write" in task: > > > > col = TASK_COLOR_PACKAGE_WRITE > > > > + elif "do_package" in task: > > > > + col = TASK_COLOR_PACKAGE > > > > else: > > > > col = WHITE > > > > > > > > > > I'm a bit torn on this as the patch changes the approach of the > > > code. > > > > > > Currently, the tasks are marked without any fuzz, i.e. it matches > > > do_compile alone. The code is entirely consistent in that all the > > > other > > > areas match specific tasks too. > > > > > > After the change you're changing the meaning to "any compile task" > > > and > > > "any configure task". > > > > > > > > > Yeah that was my intention and they still be consistent by color but > > now > > matching a sequential group of tasks: > > > > "any configure task" -> "any compile task" -> "any package task" -> > > "any package write task" -> "do_populate_sysroot" > > > > and in this way it is easier to identify the color patterns of the > > above groups, > > currently we get a couple of white bars for the majority of the > > tasks. > > > > > > > > I'm not entirely convinced this is a good thing. Should do_package > > > match do_package_qa for example? Those are quite different. I think > > > we're swapping something which is currently at least quite clear > > > for > > > something which is very fuzzy and not easy for the user to > > > understand > > > without looking at the code :/. > > > > > > > > > I can rework the patch to have the same colors as before with more > > new colors > > for the new tasks I added but with many colors It will be harder to > > understand. > > How about a compromise. Keep do_compile and "compile like" tasks > separate and use a pale version of the compile colour for "compile > like". Do something similar for configure and leave the others > unchanged? > Ok, I will do that. > > > Cheers, > > Richard >
diff --git a/scripts/pybootchartgui/pybootchartgui/draw.py b/scripts/pybootchartgui/pybootchartgui/draw.py index 3d1ff695c1..2beb3c7c67 100644 --- a/scripts/pybootchartgui/pybootchartgui/draw.py +++ b/scripts/pybootchartgui/pybootchartgui/draw.py @@ -661,20 +661,20 @@ def render_processes_chart(ctx, options, trace, curr_y, width, h, sec_w): #print("proc at %s %s %s %s" % (x, y, w, proc_h)) col = None - if task == "do_compile": + if "_setscene" in task: + col = WHITE + elif "do_compile" in task: col = TASK_COLOR_COMPILE - elif task == "do_configure": + elif "do_configure" in task: col = TASK_COLOR_CONFIGURE - elif task == "do_install": + elif "do_install" in task: col = TASK_COLOR_INSTALL - elif task == "do_populate_sysroot": + elif "do_populate_sysroot" in task: col = TASK_COLOR_SYSROOT - elif task == "do_package": - col = TASK_COLOR_PACKAGE - elif task == "do_package_write_rpm" or \ - task == "do_package_write_deb" or \ - task == "do_package_write_ipk": + elif "do_package_write" in task: col = TASK_COLOR_PACKAGE_WRITE + elif "do_package" in task: + col = TASK_COLOR_PACKAGE else: col = WHITE
This will match other deviation subtask of the same main task, a couple of them can be found on oe-core layer: do_compile_kernelmodules do_compile_ptest native_add_do_populate_sysroot_deps do_package_qa cmake_do_configure setuptools3_do_configure cargo_common_do_configure python_pyo3_do_configure python_setuptools3_rust_do_configure Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io> --- scripts/pybootchartgui/pybootchartgui/draw.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)