search

ksindi / implements

:snake: Pythonic interfaces using decorators
http://implements.readthedocs.io/
Apache License 2.0
33 stars 4 forks source link

Temporary: Do not merge #14

Closed pshirali closed 4 years ago

pshirali commented 4 years ago

This is a PR to test deepsource's config

ksindi commented 4 years ago

Thanks @pshirali! Just ping me when ready. Really appreciate the clean up.

pshirali commented 4 years ago

@ksindi There's something fishy going on with Deepsource analysis, if you look at the history here. I've actually not made any fixes to what Deepsource used to complain about in the past PRs. This PR is magically showing no issues. Not sure if new versions of Deepsource are being deployed behind the scenes, which are causing different results at different points in time.

  1. I tried changing glob patterns to first exclude tests.py, and then included it back. No change to tests.
  2. I added blank lines to implements.py and tests.py just to see if it'll force trigger a scan. Nothing happened.

Issues found with implements.py, tests.py, and docs/conf.py which were flagged earlier, didn't get flagged in recent runs. However, setup.py did get flagged, so, the analyzer is active. Not sure how/why it is being selective.

Have you seen inconsistencies in the past?

ksindi commented 4 years ago

I recently added it. I'm happy to remove it if we find it to be noisy.

pshirali commented 4 years ago

IMO, let us leave it as is. I see value in fixing some of the things which Deepsource flagged in previous runs, and excluding a few others which are intentional. It is likely that, with the current inconsistency, it may not verify my changes (or may throw something new). But hopefully over subsequent PRs we may see some improvement in what it reports. If it continues to be too noisy in the future after the cleanup, then we could take some action.

ksindi commented 4 years ago

Sounds good. Let me know when this is ready. I can release as a new minor version.

pshirali commented 4 years ago

This was a temporary branch which I was using to test deepsource's behavior. I'll decline this in a bit. I've raised another PR #15 which has all fixes. (Waiting for CI to finish. Deepsource is simply refusing to run!).