scm.py should be split into many pieces
Created attachment 91866 [details] Patch
Committed r85450: <http://trac.webkit.org/changeset/85450>
Comment on attachment 91866 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=91866&action=review > Tools/Scripts/webkitpy/common/checkout/scm/__init__.py:7 > +from .svn import SVN I mentioned this in the other bug, but since you seem to be doing this intentionally here, relative path imports are discouraged in PEP-8. > Tools/Scripts/webkitpy/common/checkout/scm/__init__.py:34 > + """ We should really add a comment here that this routine should NOT be used to determine the top of the webkit checkout, since that will only work if (a) the CWD is actually in the webkit checkout and not some other checkout and (b) the webkit checkout is a full checkout and not something like Chromium's hand-picked subset. We should point to a suggested alternative.
(In reply to comment #3) > (From update of attachment 91866 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=91866&action=review > > > Tools/Scripts/webkitpy/common/checkout/scm/__init__.py:7 > > +from .svn import SVN > > I mentioned this in the other bug, but since you seem to be doing this intentionally here, relative path imports are discouraged in PEP-8. That's what I thought, but Eric kept saying otherwise.
(In reply to comment #4) > (In reply to comment #3) > > (From update of attachment 91866 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=91866&action=review > > > > > Tools/Scripts/webkitpy/common/checkout/scm/__init__.py:7 > > > +from .svn import SVN > > > > I mentioned this in the other bug, but since you seem to be doing this intentionally here, relative path imports are discouraged in PEP-8. > > That's what I thought, but Eric kept saying otherwise. I haven't read pep-8 recently, so I don't feel an expert here. I don't think that absolute paths make sense inside module containers inside webkitpy. I think our deep module tree makes things tricky. I don't think .. relative paths *ever* make sense. But I would like to tighten up our module boundaries more. We have problems of one module reaching out into another module (checkout.changelog reaching out to bugzilla and bugzilla reaching back into checkout.scm.git for example) which I would like to get rid of. It feels a bit strange to me to have scm/__init__.py need to know where it is just to import its own .scm.py file, but if that's what the python experts tell us we should do, I'm not sure I know enough to disagree.