We do have a few Python files to do scalars/histograms parsing in the client [1]. It would be great if we could settle on a minimum set of linting rules and make all the Python files we own pass them. The rules could be shared between the client and the pipeline, as some files are shared among the services (e.g. parse_scalars.py). Mauro mentioned using flak8 to do python linting on ATMO, and provided the set of rules they are using right now [2]. Frank suggested using a global pylint file for consistency between the client and the pipeline. [1] - http://searchfox.org/mozilla-central/search?q=&case=false&regexp=false&path=telemetry%2F*.py [2] - https://github.com/mozilla/telemetry-analysis-service/blob/master/setup.cfg

Any requirements for linting we should add in the contributing doc [0]. Additionally, we should probably include our global pylint/flake8 file there, which every repo can then use for linting. Re: pylint vs. flake8, I'm okay with either. In their base configurations, pylint seems to report much more, which can be a good or bad thing, depending on how pedantic we want to be. [0] https://github.com/mozilla/telemetry-onboarding/blob/master/Contributing.md

It would also be nice if we could integrate this with the tools we already use to check our code; mainly Travis. An example for flake8 is at [0]. [0] https://github.com/frewsxcv/python-geojson/pull/46/commits/e6d3b1e0393ebf9031dea6c44b75f845aa784b4b

:gps, you know quite a bit of Python and our build system. Do you think it could be worthwhile integrating Python code linting through flake8 (or pylint) in our build system, as done with "./mach eslint"? Any comment/feedback/idea?

cc'ing ahal, as he's done a lot of work with eslint

This already exists :) Just add any directories you want here: https://dxr.mozilla.org/mozilla-central/source/tools/lint/flake8.lint#178 The default configuration lives in topsrcdir/.flake8, but you can create a new .flake8 file to change it (note: doing this will replace the default config, not update it). After adding the path, when you push to hg your results will appear in the 'f8' task in treeherder. To run locally, use: ./mach lint -l flake8 <path/to/dir> (Note: the -l flake8 is optional depending if you want to run all linters configured for that directory or just flake8) In fact, ./mach eslint is just an alias to ./mach lint -l eslint

I love when someone else steals my needinfo requests. Thank you, ahal!

@Georg, Hi, I am new to Telemetry, I would like to take up this bug. How should I proceed, I have a firefox artifact build running.

I'll redirect this to Alessio, who is the mentor for this bug.

(In reply to Deepjyoti Mondal from comment #7) > @Georg, Hi, I am new to Telemetry, I would like to take up this bug. How > should I proceed, I have a firefox artifact build running. Hi Deepijoty Mondal, for enabling Python linting we need to: 1) Add the Telemetry directory, 'toolkit/components/telemetry', at the end of https://dxr.mozilla.org/mozilla-central/source/tools/lint/flake8.lint#178 2) Run "./mach lint -l flake8 toolkit/components/telemetry" to see if our Python scripts pass the linting process with the default rules defined at https://dxr.mozilla.org/mozilla-central/source/.flake8 3) If the previous step produces no error, then we're ok and can simply land this (I don't want to spoil the fun but this is unlikely to happen :) ). If it fails, then we should create a new .flake8 file (with a structure similar to https://dxr.mozilla.org/mozilla-central/source/.flake8 ) in 'toolkit/components/telemetry' that disables the offending check/rule. We will have to file a bugs to enable these checks as follow-ups. Please do not hesitate to ask if you need more information or get stuck!

@Alessio Thanks a lot for the pointers, I shall upload a patch soon.

I added toolkit/components/telemetry in flake8.lint file but it produced lot errors on running ./mach lint -l flake8 toolkit/components/telemetry so I created a new .flake8 file at toolkit/components/telemetry at ignored the trouble causing rules. Hope this helps.

(In reply to Deepjyoti Mondal from comment #11) > Created attachment 8843378 [details] [diff] [review] > bug1332651.patch > > I added toolkit/components/telemetry in flake8.lint file but it produced lot > errors on running ./mach lint -l flake8 toolkit/components/telemetry so I > created a new .flake8 file at toolkit/components/telemetry at ignored the > trouble causing rules. Hope this helps. Thanks Deepijoty! Would you mind attaching a text file with the errors it produced?

I have attached a file containing the errors I encountered.

@Alessio Do let me know when you create the follow up issues to solve the linting errors. I would love to work on them :)

Comment on attachment 8843378 [details] [diff] [review] bug1332651.patch Review of attachment 8843378 [details] [diff] [review]: ----------------------------------------------------------------- This looks good, thanks!

Looks like the F8 job on try was successful, this is ready to land!

great :)

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/48cf9ac6ab31 Enabled python code linting in Telemetry. r=Dexter