You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/06/29 12:54:30 UTC

[GitHub] [incubator-kvrocks] tisonkun opened a new pull request, #695: ci: separate daily ci and trigger daily ci only if labeled

tisonkun opened a new pull request, #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695

   This closes #665 and closes #691.
   
   Signed-off-by: tison <wa...@gmail.com>


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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695#discussion_r910193635


##########
.github/workflows/daily-ci.yaml:
##########
@@ -19,22 +19,22 @@ name: Daily CI
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/daily-ci.yaml"
+    types: [opened, synchronize, reopened, labeled]
   schedule:
-    # Daily run this action at 1am
     - cron: '0 1 * * *'
   workflow_dispatch:
 
 jobs:
   build-docker-image:
     name: Build Docker Image
+    if: |
+      (github.event_name != 'pull_request') ||
+      (github.event.action == 'labeled' && github.event.label.name == 'needs-daily-ci') ||
+      (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'needs-daily-ci'))

Review Comment:
   If we call `daily-ci.yaml` from`kvrocks.yaml`, not only the dependencies logic will mess, but also we should now handle `pull_request` event with `labeled` type in `kvrocks.yaml`, it will cause more complex logic.
   
   In Pulsar and previous SkyWalking CI settings we do repeat some conditions as long as it won't grow too much. We can review this issue if it cause concrete trouble.



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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun merged pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
tisonkun merged PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695


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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695#discussion_r910179983


##########
.github/workflows/daily-ci.yaml:
##########
@@ -19,22 +19,22 @@ name: Daily CI
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/daily-ci.yaml"
+    types: [opened, synchronize, reopened, labeled]
   schedule:
-    # Daily run this action at 1am
     - cron: '0 1 * * *'
   workflow_dispatch:
 
 jobs:
   build-docker-image:
     name: Build Docker Image
+    if: |
+      (github.event_name != 'pull_request') ||
+      (github.event.action == 'labeled' && github.event.label.name == 'needs-daily-ci') ||
+      (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'needs-daily-ci'))

Review Comment:
   We can actually run kvrocks.yaml job everyday.



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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695#discussion_r910174145


##########
.github/workflows/daily-ci.yaml:
##########
@@ -19,22 +19,22 @@ name: Daily CI
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/daily-ci.yaml"
+    types: [opened, synchronize, reopened, labeled]
   schedule:
-    # Daily run this action at 1am
     - cron: '0 1 * * *'
   workflow_dispatch:
 
 jobs:
   build-docker-image:
     name: Build Docker Image
+    if: |
+      (github.event_name != 'pull_request') ||
+      (github.event.action == 'labeled' && github.event.label.name == 'needs-daily-ci') ||
+      (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'needs-daily-ci'))

Review Comment:
   I think the `workflow_call` method in #694 maybe better since we can have several jobs in the daily ci workflow (every job need an `if` 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.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695#discussion_r910174145


##########
.github/workflows/daily-ci.yaml:
##########
@@ -19,22 +19,22 @@ name: Daily CI
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/daily-ci.yaml"
+    types: [opened, synchronize, reopened, labeled]
   schedule:
-    # Daily run this action at 1am
     - cron: '0 1 * * *'
   workflow_dispatch:
 
 jobs:
   build-docker-image:
     name: Build Docker Image
+    if: |
+      (github.event_name != 'pull_request') ||
+      (github.event.action == 'labeled' && github.event.label.name == 'needs-daily-ci') ||
+      (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'needs-daily-ci'))

Review Comment:
   I think the `workflow_call` method in #694 maybe better since we can have several jobs in the daily ci workflow.



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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on a diff in pull request #695: ci: separate daily ci and trigger daily ci only if labeled

Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #695:
URL: https://github.com/apache/incubator-kvrocks/pull/695#discussion_r910182751


##########
.github/workflows/daily-ci.yaml:
##########
@@ -19,22 +19,22 @@ name: Daily CI
 
 on:
   pull_request:
-    paths:
-      - ".github/workflows/daily-ci.yaml"
+    types: [opened, synchronize, reopened, labeled]
   schedule:
-    # Daily run this action at 1am
     - cron: '0 1 * * *'
   workflow_dispatch:
 
 jobs:
   build-docker-image:
     name: Build Docker Image
+    if: |
+      (github.event_name != 'pull_request') ||
+      (github.event.action == 'labeled' && github.event.label.name == 'needs-daily-ci') ||
+      (github.event.action != 'labeled' && contains(github.event.pull_request.labels.*.name, 'needs-daily-ci'))

Review Comment:
   > every job need an `if` here
   
   I don't get the point. It's not a regression.



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

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org