You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@celix.apache.org by GitBox <gi...@apache.org> on 2020/05/06 13:48:03 UTC

[GitHub] [celix] abroekhuis opened a new pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

abroekhuis opened a new pull request #202:
URL: https://github.com/apache/celix/pull/202


   …not found) download via Cmake's FetchContent. Updated usage to correct alias.
   
   @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?


----------------------------------------------------------------
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] [celix] pnoltes commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616627473


   > > > …not found) download via Cmake's FetchContent. Updated usage to correct alias.
   > > 
   > > 
   > > I am seeing the same issue with find_package for CPPUTest on the feature/pubsub_custom_serializers branch. I think cpputest changed their cmake config in a recent update, but I did not investigate this yet.
   > 
   > CPPuTest has not been updated since 3.8 (2016..). So I'm not sure what is going on. I don't have any problems locally...
   
   Indeed. Maybe an update of CMake. I also see it is a warning. gtest compilation fails due to effc++ warnings. Is it possible to update the compiler flags for gtest in combination with FetchContent?


----------------------------------------------------------------
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] [celix] pnoltes commented on a change in pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #202:
URL: https://github.com/apache/celix/pull/202#discussion_r412502622



##########
File path: CMakeLists.txt
##########
@@ -41,7 +41,7 @@ ELSE ()
     set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
     set(CMAKE_CXX_FLAGS "-std=c++11 -fno-rtti ${CMAKE_CXX_FLAGS}")
     set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-Wall -Wextra -Weffc++ ${CMAKE_CXX_FLAGS}")

Review comment:
       Is it not possible to set the c/cxx flags using FetchContent?
   
   Else change the CMAKE_C_FLAGS / CMAKE_CXX_FLAGS only during FetchContent.
   
   e.g.:
   ```cmake
   set(SAVE_CXX ${CMAKE_CXX_FLAGS}
   string(REPLACE "-Weffc++" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
   FetchContent_Declare(
            googletest
            GIT_REPOSITORY https://github.com/google/googletest.git
            GIT_TAG release-1.8.1
            UPDATE_DISCONNECTED TRUE
            PREFIX ${CMAKE_BINARY_DIR}/gtest
            INSTALL_COMMAND ""
            GIT_TAG        release-1.10.0
    )
    FetchContent_MakeAvailable(googletest)
   set(CMAKE_CXX_FLAGS ${SAVE_CXX})
   unset(SAVE_CXX)
   ```
   
   Not pretty, but then we can keep effc++ warning on




----------------------------------------------------------------
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] [celix] pnoltes commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616629241


   
   > So keeping the rest of the model, what is wrong for removing develop, and use master for it instead?
   
   It is not the gitflow model. That being said, I never been a fan of both a  master and develop branch, so +1 for me for making the master the actual branch where we de development. 
   
   
   
   
   


----------------------------------------------------------------
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] [celix] pnoltes commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616351626


   > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   
   If a user does a clone checkout we want is to be the master. The latest release.
   For a PR default the develop would be better. I am not sure if this is configureable..


----------------------------------------------------------------
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] [celix] abroekhuis commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616439699


   > > > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > > > 
   > > > 
   > > > If a user does a clone checkout we want is to be the master. The latest release.
   > > > For a PR default the develop would be better. I am not sure if this is configureable..
   > > 
   > > 
   > > Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   > > Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.
   > 
   > Is it even the right way to work to use develop as main branch? We are promoting releases through the asf mirror system (they can be downloaded from GitHub as well).
   > 
   > The source repository contains our latest code. Why not use the master branch for our latest code then? It seems other Apache projects are using the master branch for their latest sources as well, e.g.: [apache/spark](https://github.com/apache/spark), [apache/dubbo](https://github.com/apache/dubbo), [apache/flink](https://github.com/apache/flink), [apache/storm](https://github.com/apache/storm), [apache/hbase](https://github.com/apache/hbase), [apache/avro](https://github.com/apache/avro). They all use the master branch for their latest sources and I'm sure other projects do the same.
   > 
   > E.g. Apache Spark also maintains `branch-*` branches for every minor release. In our case this could be a `branch-2.1` for a (possible) follow-up 2.1.1 release. Or if 3.0.0 is released and someone asks for a (quick) fix for 2.2.0?
   > 
   > EDIT: It seems we have a patch to #157 which fixes something for the 2.2.0 release. Ideally this will be released as 2.2.1.
   
   The idea to use develop makes sense when looking at git-flow. I think this is just a matter of how we want to work with the code.
   I like git-flow, but if there are other good models, with added benefits, let's discuss them!
   Basically:
   * Develop: work that (potentially) goes into a new version (not a bugfix)
   * Feature branches: active work
   * Release branch: once features are done and merged on develop, move to a release branch to stabilise. New development can continue on develop
   * Bugfix branches: bugs in releases, based on the release tag the issue is found in, results in a release, and merge to develop to get the changes in new code as well.
   * Master: stable branch with the latest release
   
   The actual usage of master can be discussed.. Since it doesn't add much. Tags point to releases, github/apache has the releases in a dedicated place.
   So keeping the rest of the model, what is wrong for removing develop, and use master for it 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] [celix] rlenferink commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
rlenferink commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616429007


   > > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > > 
   > > If a user does a clone checkout we want is to be the master. The latest release.
   > > For a PR default the develop would be better. I am not sure if this is configureable..
   > 
   > Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   > 
   > Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.
   
   Is it even the right way to work to use develop as main branch? We are promoting releases through the asf mirror system (they can be downloaded from GitHub as well). 
   
   The source repository contains our latest code. Why not use the master branch for our latest code then? It seems other Apache projects are using the master branch for their latest sources as well, e.g.: [apache/spark](https://github.com/apache/spark), [apache/dubbo](https://github.com/apache/dubbo), [apache/flink](https://github.com/apache/flink), [apache/storm](https://github.com/apache/storm), [apache/hbase](https://github.com/apache/hbase), [apache/avro](https://github.com/apache/avro). They all use the master branch for their latest sources and I'm sure other projects do the same.
   
   E.g. Apache Spark also maintains `branch-*` branches for every minor release. In our case this could be a `branch-2.1` for a (possible) follow-up 2.1.1 release. Or if 3.0.0 is released and someone asks for a (quick) fix for 2.2.0?


----------------------------------------------------------------
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] [celix] pnoltes commented on pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-618601667


   Does not work on travis (cmake too old). Should we remove travis config? 
   Note that there is an issue to move coverity scans from travis to GitHub #208 


----------------------------------------------------------------
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] [celix] rlenferink edited a comment on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
rlenferink edited a comment on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616429007


   > > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > > 
   > > If a user does a clone checkout we want is to be the master. The latest release.
   > > For a PR default the develop would be better. I am not sure if this is configureable..
   > 
   > Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   > 
   > Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.
   
   Is it even the right way to work to use develop as main branch? We are promoting releases through the asf mirror system (they can be downloaded from GitHub as well). 
   
   The source repository contains our latest code. Why not use the master branch for our latest code then? It seems other Apache projects are using the master branch for their latest sources as well, e.g.: [apache/spark](https://github.com/apache/spark), [apache/dubbo](https://github.com/apache/dubbo), [apache/flink](https://github.com/apache/flink), [apache/storm](https://github.com/apache/storm), [apache/hbase](https://github.com/apache/hbase), [apache/avro](https://github.com/apache/avro). They all use the master branch for their latest sources and I'm sure other projects do the same.
   
   E.g. Apache Spark also maintains `branch-*` branches for every minor release. In our case this could be a `branch-2.1` for a (possible) follow-up 2.1.1 release. Or if 3.0.0 is released and someone asks for a (quick) fix for 2.2.0?
   
   EDIT: It seems we have a patch to #157 which fixes something for the 2.2.0 release. Ideally this will be released as 2.2.1.
   
   EDIT 2: Also the ASF environment allows us to use an [.asf.yaml](https://s.apache.org/asfyaml) file for changing [repository settings](https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-GitHubsettings) or [notification e-mail addresses](https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Notificationsettingsforrepositories). Because the file is only accepted on the master branch we need to change the master branch anyway when we want to change any of these settings.


----------------------------------------------------------------
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] [celix] abroekhuis commented on a change in pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #202:
URL: https://github.com/apache/celix/pull/202#discussion_r421023821



##########
File path: CMakeLists.txt
##########
@@ -41,7 +41,7 @@ ELSE ()
     set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
     set(CMAKE_CXX_FLAGS "-std=c++11 -fno-rtti ${CMAKE_CXX_FLAGS}")
     set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-Wall -Wextra -Weffc++ ${CMAKE_CXX_FLAGS}")

Review comment:
       I've moved the inclusion of gtest to before the flags. This works. Since it is an external dependency, I think we can do without the flags. But.. if needed we can split up the flags (or even move Werror) so that the build does not fail due to gtest source.




----------------------------------------------------------------
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] [celix] abroekhuis commented on pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on pull request #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-618611616


   > Does not work on travis (cmake too old). Should we remove travis config?
   > Note that there is an issue to move coverity scans from travis to GitHub #208
   
   Sounds good to me. Too bad a newer cmake version is not supported.


----------------------------------------------------------------
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] [celix] abroekhuis commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616968339


   With this way of using GTest, it runs into problems when -Weffc++ is used. I have removed it from the compiler flags to be able to compile on GCC.
   
   See https://github.com/google/googletest/issues/898 for info.
   


----------------------------------------------------------------
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] [celix] abroekhuis commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616354694


   > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > 
   > If a user does a clone checkout we want is to be the master. The latest release.
   > For a PR default the develop would be better. I am not sure if this is configureable..
   
   Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   
   Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.


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



Re: [GitHub] [celix] pnoltes commented on a change in pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by Alexander Broekhuis <a....@gmail.com>.
Ok, I'll take a look. Afaik, scope of variables is couple to a cmakefile. But I'll try if I can fix thix.

--
Met vriendelijke groet,

Alexander Broekhuis
On 24 apr. 2020 09:25 +0200, GitBox <gi...@apache.org>, wrote:
>
> pnoltes commented on a change in pull request #202:
> URL: https://github.com/apache/celix/pull/202#discussion_r414354776
>
>
>
> ##########
> File path: CMakeLists.txt
> ##########
> @@ -41,7 +41,7 @@ ELSE ()
> set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
> set(CMAKE_CXX_FLAGS "-std=c++11 -fno-rtti ${CMAKE_CXX_FLAGS}")
> set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")
> - set(CMAKE_CXX_FLAGS "-Wall -Wextra -Weffc++ ${CMAKE_CXX_FLAGS}")
>
> Review comment:
> Personally I find effc++ one of the most important flags. Not always nice too see the warning after committing for OSX (clang), but almost always very useful warnings.
>
> In other words I would like to keep effc++
>
>
>
>
> ----------------------------------------------------------------
> 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] [celix] pnoltes commented on a change in pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on a change in pull request #202:
URL: https://github.com/apache/celix/pull/202#discussion_r414354776



##########
File path: CMakeLists.txt
##########
@@ -41,7 +41,7 @@ ELSE ()
     set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
     set(CMAKE_CXX_FLAGS "-std=c++11 -fno-rtti ${CMAKE_CXX_FLAGS}")
     set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-Wall -Wextra -Weffc++ ${CMAKE_CXX_FLAGS}")

Review comment:
       Personally I find effc++ one of the most important flags. Not always nice too see the warning after committing for OSX (clang), but almost always very useful warnings.
   
   In other words I would like to keep effc++




----------------------------------------------------------------
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] [celix] rlenferink edited a comment on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
rlenferink edited a comment on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616429007


   > > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > > 
   > > If a user does a clone checkout we want is to be the master. The latest release.
   > > For a PR default the develop would be better. I am not sure if this is configureable..
   > 
   > Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   > 
   > Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.
   
   Is it even the right way to work to use develop as main branch? We are promoting releases through the asf mirror system (they can be downloaded from GitHub as well). 
   
   The source repository contains our latest code. Why not use the master branch for our latest code then? It seems other Apache projects are using the master branch for their latest sources as well, e.g.: [apache/spark](https://github.com/apache/spark), [apache/dubbo](https://github.com/apache/dubbo), [apache/flink](https://github.com/apache/flink), [apache/storm](https://github.com/apache/storm), [apache/hbase](https://github.com/apache/hbase), [apache/avro](https://github.com/apache/avro). They all use the master branch for their latest sources and I'm sure other projects do the same.
   
   E.g. Apache Spark also maintains `branch-*` branches for every minor release. In our case this could be a `branch-2.1` for a (possible) follow-up 2.1.1 release. Or if 3.0.0 is released and someone asks for a (quick) fix for 2.2.0?
   
   EDIT: It seems we have a patch to #157 which fixes something for the 2.2.0 release. Ideally this will be released as 2.2.1.


----------------------------------------------------------------
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] [celix] abroekhuis commented on a change in pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on a change in pull request #202:
URL: https://github.com/apache/celix/pull/202#discussion_r414062912



##########
File path: CMakeLists.txt
##########
@@ -41,7 +41,7 @@ ELSE ()
     set(CMAKE_C_FLAGS "-D_GNU_SOURCE -std=gnu99 -fPIC ${CMAKE_C_FLAGS}")
     set(CMAKE_CXX_FLAGS "-std=c++11 -fno-rtti ${CMAKE_CXX_FLAGS}")
     set(CMAKE_C_FLAGS "-Wall -Werror ${CMAKE_C_FLAGS}")
-    set(CMAKE_CXX_FLAGS "-Wall -Wextra -Weffc++ ${CMAKE_CXX_FLAGS}")

Review comment:
       The usage off effc++ is debatable, also CLang does not have it. I don't see a big problem in not having it. Wdyt?




----------------------------------------------------------------
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] [celix] abroekhuis commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
abroekhuis commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616408638


   > > …not found) download via Cmake's FetchContent. Updated usage to correct alias.
   > 
   > I am seeing the same issue with find_package for CPPUTest on the feature/pubsub_custom_serializers branch. I think cpputest changed their cmake config in a recent update, but I did not investigate this yet.
   
   CPPuTest has not been updated since 3.8 (2016..). So I'm not sure what is going on. I don't have any problems locally...


----------------------------------------------------------------
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] [celix] pnoltes commented on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616352794


   > …not found) download via Cmake's FetchContent. Updated usage to correct alias.
   
   I am seeing the same issue with find_package for CPPUTest on the feature/pubsub_custom_serializers branch. I think cpputest changed their cmake config in a recent update, but I did not investigate this yet.
   


----------------------------------------------------------------
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] [celix] pnoltes commented on pull request #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
pnoltes commented on pull request #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-618849627


   > Does not work on travis (cmake too old). Should we remove travis config?
   > Note that there is an issue to move coverity scans from travis to GitHub #208
   
   Maybe this is addressable, but IMO travis CI is too undependable. Sometime build start immediately and sometimes there are pending for a few hours. 


----------------------------------------------------------------
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] [celix] rlenferink edited a comment on issue #202: Updated gtest setup to either use GTest supplied Config file, or (if …

Posted by GitBox <gi...@apache.org>.
rlenferink edited a comment on issue #202:
URL: https://github.com/apache/celix/pull/202#issuecomment-616429007


   > > > @pnoltes why do we not have "develop" set as main branch? Is there a reason for this, or can I change it?
   > > 
   > > If a user does a clone checkout we want is to be the master. The latest release.
   > > For a PR default the develop would be better. I am not sure if this is configureable..
   > 
   > Is that the right way to work? Releases are easily found/downloaded via the releases "Tab" here on github. I think that is the right place to point users to: https://github.com/apache/celix/releases
   > 
   > Source is for developers IMO, not users.. I think it makes much more sense to have "develop" the main branch.
   
   Is it even the right way to work to use develop as main branch? We are promoting releases through the asf mirror system (they can be downloaded from GitHub as well). 
   
   The source repository contains our latest code. Why not use the master branch for our latest code then? It seems other Apache projects are using the master branch for their latest sources as well, e.g.: [apache/spark](https://github.com/apache/spark), [apache/dubbo](https://github.com/apache/dubbo), [apache/flink](https://github.com/apache/flink), [apache/storm](https://github.com/apache/storm), [apache/hbase](https://github.com/apache/hbase), [apache/avro](https://github.com/apache/avro). They all use the master branch for their latest sources and I'm sure other projects do the same.
   
   E.g. Apache Spark also maintains `branch-*` branches for every minor release. In our case this could be a `branch-2.1` for a (possible) follow-up 2.1.1 release. Or if 3.0.0 is released and someone asks for a (quick) fix for 2.2.0?
   
   EDIT: It seems we have a patch to #157 which fixes something for the 2.2.0 release. Ideally this will be released as 2.2.1.
   
   EDIT 2: Also the ASF environment allows us to use an [.asf.yaml](https://s.apache.org/asfyaml) file for changing [repository settings](https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-GitHubsettings) or [notification e-mail addresses](https://cwiki.apache.org/confluence/display/INFRA/.asf.yaml+features+for+git+repositories#id-.asf.yamlfeaturesforgitrepositories-Notificationsettingsforrepositories). Because the file is only accepted on the master branch we need to change the master branch anyway when we want to change any of these settings. Maybe it's better to continue this as a separate topic on the dev@ list.


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