This is a step towards separating our style-checking code from the code that iterates over paths.
Created attachment 53597 [details] Proposed patch Rather than changing the working code incrementally, I decided to add the class we'll be switching to as a whole, with more or less full unit test coverage. I believe this will make the subsequent steps smaller and a bit easier to understand -- since we'll just need to start using a class that already exists and is fully tested. (I'm not sure the existing code actually tests reading files anyways. I think it tests with mock methods in all cases.)
Created attachment 53599 [details] Proposed patch 2 Added docstrings to ProcessorBase and included more docstring information elsewhere about **kwargs.
Comment on attachment 53599 [details] Proposed patch 2 Looks good! Sorry for the latency. A few comments: How about changing all single quotes to double quotes at this chance? > + class TestProcessor(ProcessorBase): How about naming this as MockProcessor?
(In reply to comment #3) > (From update of attachment 53599 [details]) > Looks good! Sorry for the latency. A few comments: Thanks! No problem -- especially since it was the weekend. > How about changing all single quotes to double quotes at this chance? I was actually making an effort to use single quotes. :) I don't think we ever decided this as a group. Personally, I started out preferring double quotes but then changed to preferring single quotes for the reason I mentioned here: http://trac.webkit.org/wiki/PythonGuidelines Also see-- https://bugs.webkit.org/show_bug.cgi?id=37062 Feel free to edit the wiki page, by the way, if you disagree or have additional thoughts. > > + class TestProcessor(ProcessorBase): > > How about naming this as MockProcessor? Sounds good.
> I was actually making an effort to use single quotes. :) I don't think we ever > decided this as a group. Personally, I started out preferring double quotes > but then changed to preferring single quotes for the reason I mentioned here: Ah, I see. I didn't notice you changed your mind :) So, let me comment again - how about changing double quotes to single quotes?
(In reply to comment #5) > > I was actually making an effort to use single quotes. :) I don't think we ever > > decided this as a group. Personally, I started out preferring double quotes > > but then changed to preferring single quotes for the reason I mentioned here: > > Ah, I see. I didn't notice you changed your mind :) So, let me comment again - > how about changing double quotes to single quotes? Did you see any spots in my patch that I missed aside from this? > + dir = os.path.join(self._temp_dir, "foo_dir") I'm starting to realize there are many distinct cases where the choice between double/single needs to be made -- not just when enclosing string literals, but also inside code comments and log messages, for example. For those I don't pretend to have a rule yet. :)
> Did you see any spots in my patch that I missed aside from this? > > > + dir = os.path.join(self._temp_dir, "foo_dir") Yeah, I agree this is the only literal which uses double-quotes.
Committed: http://trac.webkit.org/changeset/57803 (I will change the code in the above patch to match the logic discussed here: https://bugs.webkit.org/show_bug.cgi?id=37122#c21 in the patch to make this patch "live".)