It's nice to be able to run the CI manually and on push

[hunkim]

I think it's nice to be able to run the test manually and on push by adding the on event in the github action.

[2017pxy]

@hunkim Hi, thanks for your advice.

About our workflow, I think run CI on push is OK, but there is no need to set branches since we may not only develop our code on master branch. For now, the code in master branch is a stable version and in 0.2.x branch is a development version, and we will firstly add new features and new models into 0.2.x branch. Only some urgent bug fix will be merged into master branches. So my suggestion is removing the branches settings.

About running the CI manually, to be honest, I am not familiar with this feature and still confused with workflow_dispatch setting after reading some docs. But if you want to run our test manually, you can use the run_test.sh on your device to check if the code is correct before you commit it. BTW, in our development team, running the run_test.sh before committing code is a rule in our development handbook. So about this feature, I am glad to hear your more suggestion.

At last, thanks again for your kind advice and close attention to RecBole!

[hunkim]

@2017pxy Thanks for the nice reply.

About running the CI manually, to be honest, I am not familiar with this feature and still confused with workflow_dispatch setting after reading some docs.

This is quite simple. It will just add a button "Run workflow", so we could run this workflow/test manually. It's not necessary, but good to have.

BTW, in our development team, running the run_test.sh before committing code is a rule in our development handbook.

This is a very nice practice. I really like it.

However, if we add this in the PR, it's preventive checking/testing. If we test this in the push stage (and if we know this CI breaks), we need to roll back. So it's not best to have these CI checks in the push. I think it's better to have in the pull request. So we can prevent breaking changes pushed in.

Once again, I really appreciate your answer and explanation.

I don't clearly understand the branch part. Could you elaborate more on the point?

[2017pxy]

Hi, about the branches settings. I think it's better to remove setting of branches. It can be:

on: 
-push
-pull_request

BTW, I am not clear about this point:

So it's not best to have these CI checks in the push. I think it's better to have in the pull request.

Do you mean that we only need to triggle the CI test by pull request rather than push? If so, it's no need to change our settings of on. It should be:

on: 
-pull_request

[hunkim]

Thanks!

I guess branches: [ master ] indicates it runs on the only main/master branch, but I have removed the option. Please check.