search

biancadanforth / tracking-protection-shield-study

A Shield study to determine the optimal messaging, if any, for Tracking Protection in Firefox.
0 stars 3 forks source link

Run lint/build on commit using Circle-CI #43

Open pdehaan opened 6 years ago

pdehaan commented 6 years ago

I see a /.circleci/config.yml file, but wasn't seeing any CI running per commit.

We should wire up Circle to make sure we run ESLint and make sure we aren't introducing any syntax errors or whatever.

biancadanforth commented 6 years ago

This file is leftover from forking the Shield study template, where this is an old but existing issue. I agree that this is an important improvement to the shield study development process, but I would consider this an enhancement.

I may not have time to get to it for this study, but I will try!

pdehaan commented 6 years ago

Yeah, sorry. I should probably be filing a few of these upstream, since I know stuff like the npm eslint bug happens in every repo that uses the shield study template. 😿

b4handjr commented 6 years ago

Circleci would be awesome for running some UI/functional tests per commit. I could probably set this up, as well as an eslint job.

biancadanforth commented 6 years ago

Thank you @jrbenny35 for offering to do this! Right now this study does not have any addon-specific tests -- these files are stubs/relics from the template, where things are also not set up. If you would like to try and get CI working per commit with eslint and maybe with the test directory (but comment out the actual test content itself), I wouldn't say no, but I should also mention that the way we develop Shield studies is going to be changing (i.e. this may be one of the last legacy addon studies as we move toward studies implemented as WebExtension Experiments.

I don't know enough about setting up CI myself to say if it would be worth your time setting it up for a study structured as a legacy addon if it will be substantially different and require rework for a WebExtension Experiment.

Also, if you do think it could be easily re-used for a WebExtension Experiment, these changes would probably best be added as a patch to our template repo. After this study ships, I hope to start maintaining that repo (i.e. review PRs/issues and merge/fix things) on a more regular basis.

In conclusion, that is a welcome improvement, but I want to make sure your time and effort is also not wasted. What do you think given this new information?

b4handjr commented 6 years ago

@biancadanforth thank you for that explanation. I think I will explore your suggestion as well. Thanks again!