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/09/02 17:12:51 UTC

[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Ouss4 opened a new pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697


   ## Summary
   Add --list option to list all available configurations.
   Mention this option when Make is run on a not configured project.
   ## Impact
   
   ## Testing
   
   


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 edited a comment on pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685925191


   > Are you going to include configure.c changes here? Otherwise this LGTM
   
   Yes, let me do that first (when I get back to my computer).
   BTW, should we stick to short options and rename --list to -L? (-l already taken.)


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] btashton commented on pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685933525


   > > Are you going to include configure.c changes here? Otherwise this LGTM
   > 
   > Yes, let me do that first (when I get back to my computer).
   > BTW, should we stick to short options and rename --list to -L? (-l already taken.)
   
   I would prefer the short option since it seems to be what we use everywhere else.


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] btashton merged pull request #1697: tools/configure.sh: Add -L option to list all available configurations

Posted by GitBox <gi...@apache.org>.
btashton merged pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697


   


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482246222



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       Should we update configure.c and README.txt?




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482255470



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       I'm referring to the $USAGE variable, that shows how to use the tool.
   It's printed just after what used to be dumpcfgs.




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685999092


   I updated configure.c and changed the option to "-L".


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 edited a comment on pull request #1697: tools/configure.sh: Add -L option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685999092


   I updated configure.c (and configure.bat) and changed the option to "-L".


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482253804



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       > @v01d Done. Should we keep USAGE after that?
   Sorry, don't understand what you mean.

##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       > @v01d Done. Should we keep USAGE after that?
   
   Sorry, don't understand what you mean.




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482256309



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       I think it is OK to leave as is.




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482251857



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       @v01d Done.  Should we keep USAGE after that?
   
   @xiaoxiang781216 both tools/README.txt and the toplevel README.md don't detail the options, I didn't find anything to update there.
   Yes configure.c should be updated. Maybe a little bit re-worked.  For instance getopt_long will be used.




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on a change in pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#discussion_r482236801



##########
File path: tools/configure.sh
##########
@@ -134,13 +150,7 @@ if [ ! -d ${configpath} ]; then
     echo "Directory for ${boardconfig} does not exist.  Options are:"
     echo ""
     echo "Select one of the following options for <board-name>:"
-    configlist=`find ${TOPDIR}/boards -name defconfig`
-    for defconfig in ${configlist}; do
-      config=`dirname ${defconfig} | sed -e "s,${TOPDIR}/boards/,,g"`
-      boardname=`echo ${config} | cut -d'/' -f3`
-      configname=`echo ${config} | cut -d'/' -f5`
-      echo "  ${boardname}:${configname}"
-    done
+    dumpcfgs

Review comment:
       Since there's now an explicit list option wouldn't it be better to just indicate "use --list for listing boards"? this way if you mess up the parameters you don't get the long list and just the error instead




----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] v01d commented on pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685918694


   Are you going to include configure.c changes here? Otherwise this LGTM


----------------------------------------------------------------
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



[GitHub] [incubator-nuttx] Ouss4 commented on pull request #1697: tools/configure.sh: Add --list option to list all available configurations

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #1697:
URL: https://github.com/apache/incubator-nuttx/pull/1697#issuecomment-685925191


   > Are you going to include configure.c changes here? Otherwise this LGTM
   
   Yes, let me do that first (when I come back to my computer).
   BTW, should we stick to short options and rename --list to -L? (-l already taken.)


----------------------------------------------------------------
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