[GitHub] [singa] moazreyad commented on pull request #790: Remove Travis CI

[GitBox <gi...@apache.org>]

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


   > 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 `push:` trigger is enabled, this problem will be discovered when the second team member pushes the code and the workflow is triggered.


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