Continued CI discussion

[altendky]

The present CI setup doesn't cover linting or coverage etc. We can discuss general topics here until starting PRs for them.

[altendky]

@mristin, I feel like I may have jumped to a wrong conclusion. Is https://github.com/Parquery/pylddwrap/blob/160ed0aecefe59cb2d4d8a1799da5fd9fa937cdb/precommit.py related to https://pre-commit.com/? Or, is it just a file name you happened to use for a useful Python script?

[mristin]

@altendky it's just a file name I happend to use for a pre-commit script. We run it locally, on the machine, before submitting pull requests, and, prior to #16, it was run in Travis CI (see the file .travis.yml from 60b8b05).

As you can see from the file precommit.py from 60b8b05, precommit.py runs:

• YAPF, to format the code,
• Mypy, to check the types,
• Isort, to unify the imports,
• Pylint, for extra linting,
• Pydocstyle, for uniform style of the docstrings,
• Coverage, to unit test and compute the coverage,
• Doctest, to run the doctests,
• Pyicontract-lint, to lint the contracts, and
• setup.py check --restructuredtext strict to check that the Readme renders correctly on pypi.org.

I suppose these commands need to go now to tox.ini?

[altendky]

My apologies for presumably confusing earlier discussions based on my assumption about 'precommit'.

My default would be to move those into tox in some form. Separate envs, one env, I dunno. We'll see what it looks like when I get to it. I was taking care of both the kids and a busted water heater today so... As is, whatever Python is used to run precommit.py is required to already have all the utilities installed. Using tox reduces that to just one expected dependency available. Though, it does bring some overhead of creating an extra env when it may not be required given the present workflow with this repository. One way or another we could retain a single command to run all the checks in one go.

Anyways, instead of designing it all out ahead, since you seem comfortable with using tox I'll go ahead and submit a PR to add that and then we have something concrete to discuss. From there we can rework it until it is an addition you find good.

How recently was Travis actually running the tests? I didn't see any feedback from Travis in the PRs.

[mristin]

I was taking care of both the kids and a busted water heater today so...

No hurry, take your time :-)!

Separate envs, one env, I dunno.

Up to you. Since all the tests need to pass, I imagine having one dev environment makes it simpler, but I have no opinion on the matter.

How recently was Travis actually running the tests? I didn't see any feedback from Travis in the PRs.

My last pull request still ran on Travis (https://github.com/Parquery/pylddwrap/pull/13/checks, that's 2020-12-02).

[altendky]

Seems like maybe something happened with the Travis link. They've been slow for a few months, but it seems it straight up stopped building. Maybe .com vs. .org? I dunno.

The lack of CI feedback, not even a 'waiting for Travis' indicator that I'm used to, is what prompted me to jump into GHA and totally miss the existing .travis.yml. Though sure, I still usually replace Travis with GHA these days regardless. Anyways, less chat, more get-the-checks-back-running.

[altendky]

Any interest in turning on PR builds on Read the Docs? At least for me, it is encouragement to keep up on the docs and helps make sure they are getting generated properly for PRs.

[mristin]

Any interest in turning on PR builds on Read the Docs? At least for me, it is encouragement to keep up on the docs and helps make sure they are getting generated properly for PRs.

If you know how to do that, please go ahead :-). I just turn on the defaults (build docs on latest). That's probably something that needs to be tuned in .readthedocs.yml or a similar config file?

[altendky]

https://readthedocs.org/dashboard/pylddwrap/advanced/

At the link above there should be a check box labeled Build pull requests for this project. It is supposed to take care of setting up the webhooks etc. I have had to deal with it getting confused a couple times. Other work may need to be done, but that bit should give the feedback so whatever work can be done in PRs.

[mristin]

https://readthedocs.org/dashboard/pylddwrap/advanced/

At the link above there should be a check box labeled Build pull requests for this project. [...]

Thanks, the documentation is now built on every pull request!

[altendky]

So, I think I still owe coverage checks and reporting. Do you still want that going to coveralls? Do you want an in-build coverage check also? With all the troubles that I hear about and deal with with the coverage sites I'm tending more and more towards doing more of the work in-build. The sites still provide useful viewing features so I would still expect to upload to them, but... Looks like https://github.com/Bachmann1234/diff_cover might be part of the answer. With that we should be able to have failures for overall coverage level and diff coverage level. It wouldn't offer in-build project coverage delta checks (like "this PR reduces project coverage by 0.1% so fail").

Anyways, I'll get something started and we can talk through the details of what checks etc are wanted.

[mristin]

Hi @altendky ,

Do you still want that going to coveralls?

Yes, I'd say so. The users can immediately see the coverage in the form of a badge and also check which parts of the module were tested.

Do you want an in-build coverage check also?

What do you mean by "in-build coverage check"? (Sorry, I hear the term for the first time.)

If the tests are run with:

coverage run \
  --source lddwrap \
  -m unittest discover

the coverage is automatically produced.

For the report, it's:

coverage report

I think this can directly go into tox in this form?

The coveralls should be sent the coverage once all the environments ran through. See, for example, the icontract-hypothesis' test workflow: this line and that line.

[altendky]

That won't combine coverage across Python versions and interpreters nor result in a single overall report.

'in-build coverage check' was referring to having something inside the build that will cause of failure on insufficient coverage rather than depending entirely on the external service(s) to report back. Anyways, I started into it in #20. I want it somewhere else anyways so it's not wasted work if it's a problem.

[mristin]

@altendky should we actually close this issue or there are more features/problems?

[altendky]

Yeah, I think we got through most of it anyways. If there's more we can open a more specific ticket. Thanks again.