WIP: Refactor git module into a class
Changes PlannedPublic

Authored by mmodell on Nov 9 2017, 2:37 AM.

Details

Reviewers
thcipriani
demon
Group Reviewers
Release-Engineering-Team
Patch without arc
git checkout -b D874 && curl -L https://phabricator.wikimedia.org/D874?download=true | git apply
Summary

Stop passing 'location' to every method, just use self.location

Test Plan

currently untested and callsites have not been updated yet.

Diff Detail

Repository
rMSCA Scap
Branch
git-refactor (branched from master)
Lint
Lint OK
Unit
Unit Tests OK
Build Status
Buildable 2522
Build 4159: docker-diffsJenkins
Build 4158: differential-jessieJenkins
Build 4157: arc lint + arc unit
mmodell created this revision.Nov 9 2017, 2:37 AM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald TranscriptNov 9 2017, 2:37 AM
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript

Ugh. this diff doesn't make it clear what changed because every line changes at least a little bit. I'm not sure how to make reviewing this easier.

demon added a comment.Nov 9 2017, 3:31 AM

Maybe introduce the class and like one method to start? Might get the framework in place to allow us to swap other methods in a more diff-able manner?

demon added inline comments.Nov 9 2017, 3:36 AM
scap/git.py
577

I'm wondering also if we should give the option to let subsequent calls return the string command instead of actually executing: could be useful for things we want to pass to ssh.Command() instead of actually executing directly.

mmodell added inline comments.Nov 10 2017, 1:34 AM
scap/git.py
577

That's not a bad idea. The command class has a tostring (and tounicode) method but it'd be nice if there was an easy way to return the args as an array instead of a concatenated string. It would be pretty easy to add that I think.

In D874#17453, @demon wrote:

Maybe introduce the class and like one method to start? Might get the framework in place to allow us to swap other methods in a more diff-able manner?

I'll try it.

mmodell updated this revision to Diff 2329.Nov 16 2017, 7:19 PM

Do this more incrementally. Now the Repository class simply calls the old non-class methods.

mmodell planned changes to this revision.Feb 22 2018, 12:23 AM