openwisp-utils
openwisp-utils copied to clipboard
[qa] Does commit check work on github actions?
Looking at the current commit message check, I see travis being mentioned a few times:
https://github.com/openwisp/openwisp-utils/blob/30ad216d6e12a7bf4683f131967e763dbcc8363c/openwisp-qa-check#L104-L106
https://github.com/openwisp/openwisp-utils/blob/30ad216d6e12a7bf4683f131967e763dbcc8363c/openwisp-qa-check#L143
I wonder how this impacts on the builds on github actions. @pandafy @atb00ker any suggestion?
The commit check works on GitHub Actions because @atb00ker made commitcheck run every time in https://github.com/openwisp/openwisp-utils/commit/fb3ffcc2c3c42dd065d93248d87433433de31421. Running on GitHub Actions is equivalent to running on a local machine for the script.
@pandafy However,
if [ "$TRAVIS" = true ] && [ -z "$TRAVIS_PULL_REQUEST" ]; then
echo "SKIPPED: Commit message check skipped!"
fi
this condition was added because when you create a merge commit, it fails on master, so we would need an equivalent condition to don't run on openwisp-[repo]/master! :smile:
Also, I think we shouldn't remove the Travis conditions but just add the GitHub actions conditions so that travis remains usable! (Still required for docker-openwisp, moving that one is hard because building docker images in CI is 💰 and not everyone allows it, I know it's not easy on Gitlab! 😅 )
@atb00ker if I am not wrong, cases like merge commits are handled here https://github.com/openwisp/openwisp-utils/blob/7d9dfa1be17e5bb8893c0d05667dc8cd913271f8/openwisp_utils/qa.py#L92-L103
@devkapilbansal this is rather an old comment and the process has been changed since, please ignore the comment if it is not valid.
Bottom line is we need to ensure the tests run when:
- We test on CI
- We test on Local
And they don't fail because of merge commits etc! :smile:
Hello, Is there anyone working on this issue if not then can you assign this issue to Me?
Hi @Vihar214 ,
You don't need to ask anyone to assign it, you can just work on the issue and create a PR. Someone should look at it soon!
P.S: This ticket is rather old, if it is outdated and not required anymore, please share your findings. 😄