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/07/01 03:27:39 UTC

[GitHub] [incubator-nuttx-apps] xiaoxiang781216 opened a new pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

xiaoxiang781216 opened a new pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316


   since it is moved to the central place in nuttx side instead
   
   Signed-off-by: Xiang Xiao <xi...@xiaomi.com>
   Change-Id: I544d6110f1ca6460f7c82f970870aa9b1e7ab3dd
   
   ## Summary
   
   ## 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-apps] xiaoxiang781216 commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652528829


   @davids5 the change is compatable as before since all config which utilize the global constructor should already define CONFIG_HAVE_CXXINITIALIZE. The user don't take any special action and the feature still work as it, the only difference is that some config doesn't need anymore(e.g. CONFIG_EXAMPLES_HELLOXX_CXXINITIALIZE or SYSTEM_NSH_CXXINITIALIZE), but there isn't any bad side effect if user forget to remove these config from their defconfig file.


----------------------------------------------------------------
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-apps] patacongo commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652507257


   @xiaoxiang781216 I will go ahead and merge this now.  it still cannot pass the PR checks  because some defconfig files set appliation C++ settings that no longer exist, for example.  esp32-core/nsh
   
   This will break all all PR checks until all of the relevant defconfig files are updated in the incubator_nuttx repository.


----------------------------------------------------------------
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-apps] xiaoxiang781216 commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652418299


   > This should NOT me merged [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341) is stalled. It is the wrong approach .
   
   Yes, we shouldn't merge this patch before [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341). I provide this patch just show the current siutation as reference. Whether this is wrong approach is still in the discussion, please give you comment in [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341) 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-apps] patacongo merged pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

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


   


----------------------------------------------------------------
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-apps] xiaoxiang781216 commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652513951


   Yes, here is the patch: https://github.com/apache/incubator-nuttx/pull/1347


----------------------------------------------------------------
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-apps] davids5 commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652522526


   @xiaoxiang781216 would you be kind enough to fill in the #Impact section and give porting instruction? 


----------------------------------------------------------------
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-apps] patacongo commented on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652508861


   > This will break all all PR checks until all of the relevant defconfig files are updated in the incubator_nuttx repository.
   
   I will submit the the PR that fixes the PR checks in a few minutes.


----------------------------------------------------------------
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-apps] xiaoxiang781216 edited a comment on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652418299


   > This should NOT me merged [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341) is stalled. It is the wrong approach .
   
   Yes, we shouldn't merge this patch before [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341). I provide this patch just show the current siutation as reference. Whether this is a wrong approach is still in the discussion, please give you comment in [apache/incubator-nuttx#1341](https://github.com/apache/incubator-nuttx/pull/1341) 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-apps] xiaoxiang781216 edited a comment on pull request #316: apps: Remove all stuff related to CONFIG_xxx_CXXINITIALIZE

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #316:
URL: https://github.com/apache/incubator-nuttx-apps/pull/316#issuecomment-652528829


   @davids5 the change is compatable as before since all config which utilize the global constructor should already define CONFIG_HAVE_CXXINITIALIZE. The user don't need take any special action and the feature still work as it, the only difference is that some config doesn't need anymore(e.g. CONFIG_EXAMPLES_HELLOXX_CXXINITIALIZE or SYSTEM_NSH_CXXINITIALIZE), but there isn't any bad side effect if user forget to remove these config from their defconfig file.


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