You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/04/01 04:24:05 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #677:
testbuild.sh: Add an option to show the list of configs
yamt opened a new pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677
This was useful for me when tweaking testlists.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401363755
##########
File path: tools/testbuild.sh
##########
@@ -92,6 +94,9 @@ while [ ! -z "$1" ]; do
shift
nuttx="$1"
;;
+ -p )
+ PRINTLISTONLY=1
Review comment:
why? the majority of variables in this script are with upper case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401362904
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
We need monitor configs we skip carefully and reduce them as much as possible, so this log is useful.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401357597
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
+ return
+ fi
echo "Skipping: $1"
else
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
Move after line 220?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401363261
##########
File path: tools/testbuild.sh
##########
@@ -92,6 +94,9 @@ while [ ! -z "$1" ]; do
shift
nuttx="$1"
;;
+ -p )
+ PRINTLISTONLY=1
Review comment:
Should we use lower case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401360814
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
because it was useless for my purpose.
if you have a use case, i can add it back.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401359861
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
+ return
+ fi
echo "Skipping: $1"
else
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
why?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401364956
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
+ return
+ fi
echo "Skipping: $1"
else
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
i wanted machine-readable output. maybe it's a matter of preference.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401356775
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
Review comment:
Should we add a function to avoid the code dup here and line 281?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401363060
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
Review comment:
Yes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401357409
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
Why don't print the skip entry? it's also important to monitor and reduce them.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401365852
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
+ return
+ fi
echo "Skipping: $1"
else
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
i reverted the output changes to keep the change minimum. i guess you prefer this version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #677:
testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401361619
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
Review comment:
do you mean something like this?
function print_separator {
if [ ${PRINTLISTONLY} -ne 1 ]; then
echo "===================================================================================="
fi
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #677: testbuild.sh: Add an option to show the list of configs
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #677: testbuild.sh: Add an option to show the list of configs
URL: https://github.com/apache/incubator-nuttx/pull/677#discussion_r401362317
##########
File path: tools/testbuild.sh
##########
@@ -196,12 +201,21 @@ function build {
# Coordinate the steps for the next build test
function dotest {
- echo "===================================================================================="
+ if [ ${PRINTLISTONLY} -ne 1 ]; then
+ echo "===================================================================================="
+ fi
config=`echo $1 | cut -d',' -f1`
re="-${config/\//:}[[:space:]]"
if [[ "${blacklist} " =~ $re ]]; then
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
+ return
+ fi
echo "Skipping: $1"
else
+ if [ ${PRINTLISTONLY} -eq 1 ]; then
Review comment:
Line 219 already print the config name, it doesn't make sense to dup the code here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services