singa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [singa] chrishkchris commented on pull request #790: Remove Travis CI
Date Tue, 08 Sep 2020 07:09:53 GMT

chrishkchris commented on pull request #790:
URL: https://github.com/apache/singa/pull/790#issuecomment-688666652


   > > Would it be a better practice if we use the merged code instead of the PR branch
for CI test?
   > 
   > It is possible to remove `pull_request:` from the trigger events in the [workflow
yaml files](https://github.com/apache/singa/tree/dev/.github/workflows):
   > 
   > ```
   > on:
   >   push:
   >   pull_request:
   > ```
   > 
   > or we can keep it as a guide for the PR reviewer but not requiring all PR to pass
all the tests. It will be up to the PR reviewer to decide if the failed tests should be fixed
in this PR or not. I would propose to keep it because sometimes it is better to discover the
failed tests in the PR before the merge. But all the team can vote on the best options for
triggering the workflow. There are many [events for workflow trigger](https://docs.github.com/en/actions/reference/events-that-trigger-workflows)
and may be some of them can be enabled on some specific workflows.
   > 
   > > Also another problem is that if team members A and B working on the same branch,
their code can pass the CI test separately, but may not be compatible to each other (e.g.
due to API change)
   > 
   > If the is enabled, this problem will be discovered when the second team member pushes
the code and the workflow is triggered.
   
   Thanks a lot for the opinion. I also think that keeping all the test is good, as it helps
us to safeguard the code base.
   
   I suggest using the merged code (i.e. the code merging from PR branch and dev branch) for
CI test instead of using PR branch code only for CI test, the flow I suggest is something
like this:
   Step 1: Merged_code = Merge (PR branch, base branch)
   Step 2: Test_result = CI_Test(Merged_code)
   Step 3: Make decision whether to merge the code based on Test_result.
   
   Maybe this is equivalent to the `push:` trigger but I am not sure.


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



Mime
View raw message