Enable fussy fox check suite #237

codingjoe · 2019-03-25T20:38:39Z

Enable Fussy Fox check suite in favor of Travis-CI QA runs. This will give contributors feedback on minor code issues faster and more conveniently.

jezdez · 2019-03-25T21:15:38Z

@codingjoe Any chance to see the output from the individual checks?

jezdez · 2019-03-25T21:18:14Z

@codingjoe Ah nevermind, can you create the GitHub check run without the link to https://fussyfox.github.io/ so the "Details" link goes to the check tab instead?

codingjoe · 2019-03-26T07:01:21Z

Ah nevermind, can you create the GitHub check run without the link to https://fussyfox.github.io/ so the "Details" link goes to the check tab instead?

So funnily, we don't actually provide any URL, this seems to default to the website URL of the GitHub app.
https://github.com/FussyFox/lintipy/blob/9e7cea995895bbca0051bec455fcf2534eb8b652/lintipy.py#L259-L273
However, I could supply an explicit URL to this. I don't really have a better idea. The idea is to have it completely integrated with GitHub and keep external resources to computing only.

codecov · 2019-03-26T07:28:39Z

Codecov Report

Merging #237 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   89.86%   89.86%           
=======================================
  Files          25       25           
  Lines        1194     1194           
  Branches      106      106           
=======================================
  Hits         1073     1073           
  Misses         89       89           
  Partials       32       32

Flag	Coverage Δ
#coverage	`89.86% <ø> (ø)`	⬆️
#dj111	`89.86% <ø> (ø)`	⬆️
#dj20	`88.52% <ø> (ø)`	⬆️
#dj21	`88.35% <ø> (ø)`	⬆️
#py27	`89.19% <ø> (ø)`	⬆️
#py34	`88.52% <ø> (ø)`	⬆️
#py35	`88.52% <ø> (ø)`	⬆️
#py36	`88.52% <ø> (ø)`	⬆️
#py37	`88.52% <ø> (ø)`	⬆️
#pypy	`89.19% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f887ad5...ec8a8e7. Read the comment docs.

codecov · 2019-03-26T07:28:39Z

Codecov Report

Merging #237 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   89.86%   89.86%           
=======================================
  Files          25       25           
  Lines        1194     1194           
  Branches      106      106           
=======================================
  Hits         1073     1073           
  Misses         89       89           
  Partials       32       32

Flag	Coverage Δ
#coverage	`89.86% <ø> (ø)`	⬆️
#dj111	`89.86% <ø> (ø)`	⬆️
#dj20	`88.52% <ø> (ø)`	⬆️
#dj21	`88.35% <ø> (ø)`	⬆️
#py27	`89.19% <ø> (ø)`	⬆️
#py34	`88.52% <ø> (ø)`	⬆️
#py35	`88.52% <ø> (ø)`	⬆️
#py36	`88.52% <ø> (ø)`	⬆️
#py37	`88.52% <ø> (ø)`	⬆️
#pypy	`89.19% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f887ad5...12036fa. Read the comment docs.

.bandit

.checks.yml

tox.ini

Add support to run QA tests locally

jezdez · 2019-03-26T10:39:33Z

Ah nevermind, can you create the GitHub check run without the link to fussyfox.github.io so the "Details" link goes to the check tab instead?

So funnily, we don't actually provide any URL, this seems to default to the website URL of the GitHub app.
FussyFox/lintipy:lintipy.py@9e7cea9#L259-L273
However, I could supply an explicit URL to this. I don't really have a better idea. The idea is to have it completely integrated with GitHub and keep external resources to computing only.

How about uploading the log output to S3 and linking to it in the PR? That should be relatively cheap and you could purge them when the PR is merged?

codingjoe · 2019-03-26T11:03:25Z

How about uploading the log output to S3 and linking to it in the PR? That should be relatively cheap and you could purge them when the PR is merged?

We did that, before there was GitHub checks. To be honest, in terms of privacy I prefer the option to have it on GitHub only. If you have it on S3, you need to the the ACL policy to public read. Because you will need to provide a permanent link. This would add loads more of security concerns, where in terms of features, it's better suited to have the information without even leaving GitHub.
So given that I have limited time, I would rather spend it, on adding NodeJS support or annotation support, so that linter leave proper code annotations.

jezdez · 2019-03-26T11:21:34Z

How about uploading the log output to S3 and linking to it in the PR? That should be relatively cheap and you could purge them when the PR is merged?

We did that, before there was GitHub checks. To be honest, in terms of privacy I prefer the option to have it on GitHub only. If you have it on S3, you need to the the ACL policy to public read. Because you will need to provide a permanent link. This would add loads more of security concerns, where in terms of features, it's better suited to have the information without even leaving GitHub.
So given that I have limited time, I would rather spend it, on adding NodeJS support or annotation support, so that linter leave proper code annotations.

Sure, just a suggestion :) It seems to correctly link to the checks tab now btw! That is enough for me, tbh.

codingjoe · 2019-03-29T07:29:29Z

Cool, so we are good here?

jezdez · 2019-03-29T13:50:11Z

@codingjoe No, I suggested to not use the generic file name .checks.yml but instead .fussyfox.yml. Could you please implement this to prevent confusion?

codingjoe · 2019-04-01T07:16:48Z

@codingjoe No, I suggested to not use the generic file name .checks.yml but instead .fussyfox.yml. Could you please implement this to prevent confusion?

I can't simply change the name, the tool has already too many users. I also don't know of anyone using the name .checks.yml. However, I could add support for multiple names. Would that solve the issue?

jezdez · 2019-04-01T08:28:43Z

@codingjoe No, I suggested to not use the generic file name .checks.yml but instead .fussyfox.yml. Could you please implement this to prevent confusion?

I can't simply change the name, the tool has already too many users. I also don't know of anyone using the name .checks.yml. However, I could add support for multiple names. Would that solve the issue?

Sure, I'd prefer using the more specific name, despite the tool being used already, I think it's better long term. So using multiple ones seem like a sensible strategy.

codingjoe · 2019-04-01T12:15:53Z

Working on it... FussyFox/suite#24

codingjoe · 2019-04-01T13:02:42Z

Done!

tox.ini

Co-Authored-By: codingjoe <[email protected]>

codingjoe changed the title ~~Enable fussy fox check suite~~ WIP - Enable fussy fox check suite Mar 26, 2019

codingjoe added 2 commits March 26, 2019 08:15

Enable check suite in favor of travis-ci QA runs

b754256

Fix bandit and flake8 errors

ec8a8e7

codingjoe force-pushed the checks branch from 49da478 to ec8a8e7 Compare March 26, 2019 07:16

codingjoe requested a review from jezdez March 26, 2019 07:18

jazzband deleted a comment from codecov bot Mar 26, 2019

codingjoe changed the title ~~WIP - Enable fussy fox check suite~~ Enable fussy fox check suite Mar 26, 2019

jazzband deleted a comment from codecov bot Mar 26, 2019

blueyed reviewed Mar 26, 2019

View reviewed changes

.bandit Outdated Show resolved Hide resolved

.checks.yml Outdated Show resolved Hide resolved

tox.ini Show resolved Hide resolved

Add checkqa test environment to tox

934ca95

Add support to run QA tests locally

codingjoe requested a review from blueyed March 29, 2019 07:24

Rename .checks.yml to .fussyfox.yml

bda8c22

amureki reviewed Apr 1, 2019

View reviewed changes

tox.ini Outdated Show resolved Hide resolved

Fix typo

12036fa

Co-Authored-By: codingjoe <[email protected]>

jezdez approved these changes Apr 1, 2019

View reviewed changes

jezdez merged commit be2a0dc into master Apr 1, 2019

jezdez deleted the checks branch April 1, 2019 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable fussy fox check suite #237

Enable fussy fox check suite #237

codingjoe commented Mar 25, 2019 •

edited

Loading

jezdez commented Mar 25, 2019

jezdez commented Mar 25, 2019

codingjoe commented Mar 26, 2019

codecov bot commented Mar 26, 2019

codecov bot commented Mar 26, 2019 •

edited

Loading

jezdez commented Mar 26, 2019

codingjoe commented Mar 26, 2019

jezdez commented Mar 26, 2019

codingjoe commented Mar 29, 2019

jezdez commented Mar 29, 2019

codingjoe commented Apr 1, 2019

jezdez commented Apr 1, 2019

codingjoe commented Apr 1, 2019

codingjoe commented Apr 1, 2019

Enable fussy fox check suite #237

Enable fussy fox check suite #237

Conversation

codingjoe commented Mar 25, 2019 • edited Loading

jezdez commented Mar 25, 2019

jezdez commented Mar 25, 2019

codingjoe commented Mar 26, 2019

codecov bot commented Mar 26, 2019

Codecov Report

codecov bot commented Mar 26, 2019 • edited Loading

Codecov Report

jezdez commented Mar 26, 2019

codingjoe commented Mar 26, 2019

jezdez commented Mar 26, 2019

codingjoe commented Mar 29, 2019

jezdez commented Mar 29, 2019

codingjoe commented Apr 1, 2019

jezdez commented Apr 1, 2019

codingjoe commented Apr 1, 2019

codingjoe commented Apr 1, 2019

codingjoe commented Mar 25, 2019 •

edited

Loading

codecov bot commented Mar 26, 2019 •

edited

Loading