1801606 - Please allow pre-commit.ci on the mozilla-mobile Github organization

Status: RESOLVED FIXED

(mozilla.org :: Github: Administration, task)

Description

[Johan Lorenzo [:jlorenzo] - PTO - Back August 24th]

Hello there!

I'm currently working on bug 1801600 to clean up some Python code[1] used to automate some tedious tasks like bumping Geckoview. pre-commit is a tool to make git pre-commit hooks easier to interface with well-known tools like linters. For instance, you can plug Python's black[3] to automatically format the code before the commit it made.

pre-commit.ci[4] is an additional online tool which does 2 things:

1. it keeps pre-commit regularly up to date by creating pull-requests[5]
2. it monitors pull-requests to fix files in case the PR author didn't run pre-commit locally[6]

This morning, I requested this access and :cknowles redirected me here because pre-commit has never been approved on mozilla-mobile.

[1] https://github.com/mozilla-mobile/relbot
[2] https://pre-commit.com
[3] https://github.com/psf/black
[4] https://pre-commit.ci/
[5] e.g. https://github.com/taskcluster/taskgraph/pull/153
[6] e.g. https://github.com/taskcluster/taskgraph/pull/126/commits/3b4a8e8ae8858f4bbba29b7f94a2197c72796599

Comment 1

[Chris Knowles [:cknowles]]

Alright, this is a new app for the mozilla-mobile org, and it's not in the pre-approved list here so I have to route this to the secops folk.

Austin, can you take a look and weigh in on this? Thanks.

Comment 2

[Austin Sargent]

:jlorenzo Thanks for reaching out, what is the protection against someone adding a rogue hook or addition to code that this tool may automatically commit?

Comment 3

[Johan Lorenzo [:jlorenzo] - PTO - Back August 24th]

Great question! The configuration is entirely stored in the git repository. This means any addition follows the same process as any piece of code the given git repository holds. Therefore, if there is a review process on the given git repository, then the reviewer and the author are in charge of making sure the PR looks valid and doesn't do anything suspicious.

More over, if we notice something wrong after the facts, the git history is here to trace when that happened and what PR caused it.

Let me know if I can provide any additional info.

Comment 4

[Austin Sargent]

NIing :hwine as he may have some additional questions.

Comment 5

[hwine]

So, the first part I missed on my first read was that the app is auto-committing only to the PR, not a production branch. (Situation changes slightly if there are more than one production branches.)

The second issue is around the auto-generated-PR for pre-commit plugin updates. But the automation actually makes more visible what happens in a manual pre-commit autoupdate, so that's good.

The larger challenge with pre-commit is that there is no 'vetting' of plugins, and the dependencies aren't processed by any tool I know of. But this challenge is not changed by use of the pre-commit.ci app.

:jlorenzo - not a blocker or requirement, but adding .pre-commit-config.yaml to CODEOWNERS protection would be something to consider.

NI: asargent to handle the approval details

Comment 6

[Austin Sargent]

:jlorenzo - Apologies on the delay, which repos in particular would you need this app approved for? We'd like to limit it if possible, but it can be enabled for Mozilla-Mobile if there is a strong enough use case.

Comment 7

[Johan Lorenzo [:jlorenzo] - PTO - Back August 24th]

No worries! Let's start with https://github.com/mozilla-mobile/relbot. As far as I know, there is no plan to use pre-commit in any other mobile repos.

Comment 8

[Austin Sargent]

:cknowles, can you approve this app for https://github.com/mozilla-mobile/relbot? I will update the documentation/tables within the week.

Comment 9

[Chris Knowles [:cknowles]]

Alright, that button has been clicked. :jlorenzo, things should be working, or at least able to be authorized to work - let us know if there are any problems or concerns.

Comment 10

[Johan Lorenzo [:jlorenzo] - PTO - Back August 24th]

Attachment: [mozilla-mobile/relbot] pre-commit.ci tweaks now that it's enabled

:jlorenzo, things should be working

I confirm it works! I just have to make a few adjustments to green it up (see attached PR)

:jlorenzo - not a blocker or requirement, but adding .pre-commit-config.yaml to CODEOWNERS protection would be something to consider.

Good call! Done in the attached PR 🙂