Use GitHub Actions

[altendky]

Draft for:

• Conclusion on the flake8 status
  • Should it be failing with thousands of errors? Or am I running it wrong...
  • Presently it is set to allow failure. This allows us to see the output without having it's failure mask the failure of the rest of the build. Travis wasn't ever actually running flake8 so I think this is acceptable for an initial switch to GitHub Actions.
• Conclusion on Coveralls
  • Coveralls doesn't support PRs from forks via GitHub Actions. One solution is to just put the Coveralls repository token into the repository. Trio and Twisted have done similar, for example, with the reasoning being that an evil actor uploading coverage reports isn't the scariest of attack vectors for a project. Or, if you aren't super attached to Coveralls, Codecov has a GitHub app that deals with it.
  • Coveralls will be added in 596.

[dhoomakethu]

@altendky this looks good. Would you mind take a look in to the failure cases please?

[altendky]

@dhoomakethu, yep, working on it. Note that the PR is marked draft as it is not ready. :]

[dhoomakethu]

looks like the tests failed on linux for missing .coverage file and on windows. Btw, its good to have windows tests in place but I can live with out it for the time being.

[altendky]

Yes, I tossed them all in. I'll see what I can get working by just correcting the CI test setup as opposed to what needs 'real' fixes that should be addressed in a separate PR.

[dhoomakethu]

I have merged all your other PR's in to branch 2.5.0 and are now part of #578 . Thanks for the contributions.

[altendky]

Getting closer, need to review some various tidbits and todos...

[altendky]

@dhoomakethu, alrighty, as listed in the first comment there are two reasons I have this still as a draft. It's almost there except for the flake8 and Coveralls questions. I am fine discussion them on either Gitter or here, whatever you like. Also, there are follow-up PRs for enabling Windows (#593) and macOS (#594) testing. They may take a bit more 'real code' changes. The code changes I included here seemed trivial enough to be ok.

If any of this doesn't follow the spirit of the pymodbus project setup, don't hesitate to mention it. When I start using new projects I tend to end up doing various cleanup (like this CI PR) and try to get a feel for how the project likes to operate and be configured etc. I'll have my opinions, but I can certainly make adjustments to make my changes fit better. :]

[altendky]

@dhoomakethu, alrighty, I think this is at least worth looking over now. One thing that GitHub Actions is lacking is yaml anchors or other tooling for reusing various bits. A result of this is a fair bit of duplication between the job matrices. I don't like this, I have accepted it after the various setups I have done. The extensive and sometimes repetitive definitions are another thing I have accepted that aren't representative of my normal approach.

          - name: CPython 2.7
            tox: py27
            action: 2.7
            docker: 2.7
            implementation: cpython

I've concluded that it is simpler to just repeat each needed value there for it's target use. Admittedly, I often work with more complicated matrices (os/python version/python interpreter/qt library/qt version/and sometimes another one or two...) and end up applying this sometimes beyond what is needed in the simpler cases. Also, some of the jobs in the config could be made simpler but I have chosen to keep them similar to each other instead.

The Docker images for Linux are a habit I've made after I ran into some issues that were _only_ reproducible in GitHub Actions native Linux VMs. If GHA supported Windows and macOS Docker images I would probably do that too. But, it's likely not important here.

Anyways, given that I am new to this project I thought I'd give some context for this PR. I am happy to make changes or further discuss any topics in this or any other venue you prefer.

[altendky]

Kick CI to make sure this is still working 'well'.

[altendky]

I would generally recommend lowering the limit for coverage so that it passes. Using a global coverage limit to mark a given PR as passing or failing isn't super useful. It doesn't indicate if that PR has successfully covered their own changes or not so it says very little about the PR. Rather, it blocks the indication of success on every other front. Codecov (and I assume coveralls) can provide a status for the coverage of the patch which does speak directly to the PR.

That said, if you want increased coverage then you can create a PR to bump the required coverage up to whatever level you want and work on it until you surpass that level.

[altendky]

Looks like there is a small thing to fix.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[altendky]

@dhoomakethu, is there anything I should work on here? If not I can be patient until you have time. :]

[dhoomakethu]

@altendky, the tests on 3.7 are failing not sure if thats Ok, I was holding this up because of that. Thought you were working on it.

[dhoomakethu]

never mind, I have the fix for that. I will merge this in to dev this week. Thanks for your patience.

[dhoomakethu]

@altendky this is merged and is part of #605. I will be closing this.