You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/12/18 10:50:40 UTC

[GitHub] [nifi-minifi-cpp] hunyadi-dev opened a new pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

hunyadi-dev opened a new pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966


   Cherry picking the civetweb fix onto the release candidate 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



[GitHub] [nifi-minifi-cpp] fgerlits commented on pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
fgerlits commented on pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966#issuecomment-753903736


   You will also need to cherry-pick #925 because of the `set-env` deprecation in GitHub Actions.


----------------------------------------------------------------
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] [nifi-minifi-cpp] fgerlits commented on pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
fgerlits commented on pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966#issuecomment-753903252


   Same as #952, which is already in main, so should be safe to merge.


----------------------------------------------------------------
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] [nifi-minifi-cpp] arpadboda commented on pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
arpadboda commented on pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966#issuecomment-753985192


   Merged in scope of https://github.com/apache/nifi-minifi-cpp/commit/24d16c1c92d1bf818e5cbba7e91f7eb0688dfcb2


----------------------------------------------------------------
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] [nifi-minifi-cpp] arpadboda closed pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
arpadboda closed pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966


   


----------------------------------------------------------------
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] [nifi-minifi-cpp] fgerlits commented on a change in pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966#discussion_r551273890



##########
File path: .github/workflows/ci.yml
##########
@@ -259,8 +249,7 @@ jobs:
             ubuntu-16.04-shared-ccache-refs/heads/main-
       - id: install_deps
         run: |
-          echo "::set-env name=CMAKE_BUILD_OPTIONS::-DENABLE_PCAP=TRUE"
           sudo apt install -y ccache
-          echo "::set-env name=PATH::/usr/lib/ccache:$PATH"
+          echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
       - id: build
         run: ./bootstrap.sh -e -t && cd build  && cmake -DUSE_SHARED_LIBS=ON -DSTRICT_GSL_CHECKS=AUDIT .. &&  cmake --build . --parallel 4  && make test ARGS="--timeout 300 -j4 --output-on-failure"

Review comment:
       It looks like you manually removed the docker timeout change from this cherry-picked commit.  Why?
   
   EDIT: never mind, that docker test does not exist on this branch, 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] [nifi-minifi-cpp] fgerlits commented on a change in pull request #966: MINIFICPP-1405 - Update civetweb version, remove patch, disable building lua and duktape deps

Posted by GitBox <gi...@apache.org>.
fgerlits commented on a change in pull request #966:
URL: https://github.com/apache/nifi-minifi-cpp/pull/966#discussion_r551273890



##########
File path: .github/workflows/ci.yml
##########
@@ -259,8 +249,7 @@ jobs:
             ubuntu-16.04-shared-ccache-refs/heads/main-
       - id: install_deps
         run: |
-          echo "::set-env name=CMAKE_BUILD_OPTIONS::-DENABLE_PCAP=TRUE"
           sudo apt install -y ccache
-          echo "::set-env name=PATH::/usr/lib/ccache:$PATH"
+          echo "PATH=/usr/lib/ccache:$PATH" >> $GITHUB_ENV
       - id: build
         run: ./bootstrap.sh -e -t && cd build  && cmake -DUSE_SHARED_LIBS=ON -DSTRICT_GSL_CHECKS=AUDIT .. &&  cmake --build . --parallel 4  && make test ARGS="--timeout 300 -j4 --output-on-failure"

Review comment:
       It looks like you manually removed the docker timeout change from this cherry-picked commit.  Why?
   
   It's OK if you have a good reason, but there is a danger that this change will be accidentally reverted when this branch is merged back to main.
   
   Also, if you remove this part of the commit, please remove it from the commit message, too -- or, even better, mention in the commit message that you have removed this.




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