The DarwinPort class in webkitpy calls PlistBuddy to extract the app identifier from the bundle of an app. This function should be memoized to prevent unneeded calls to PlistBuddy.
<rdar://problem/30518832>
Created attachment 301538 [details] Patch
Comment on attachment 301538 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301538&action=review > Tools/ChangeLog:7 > + Please explain why we are making these changes. I assume we are making the memorization change because we instantiate SimulatorProcess frequently and SimulatorProcess.__init__() computes the app bundle identifier. I am assuming DarwinPort._get_crash_log() was just wrong? Regardless, please clarify. The purpose of the ChangeLog is to explain why a change was made. The only time it is acceptable to omit a description is when the title sufficiently explains the reason for a change. The title of this bug is insuffient as it only explains what we are doing and not why. On another note, we generally prefer one bug fix per patch. Among other reasons this limits the risk of causing a regression that could result in a full rollout of the patch. When the patch fixes multiple bugs a rollout in response to a regressiion due to one of the bug fixes results in the undoing of the unrelated bug fixes (since they are in the same patch and assuming the person that performs rollout does a full revert). Using the one bug fix per patch approach at most one of the bug fixes would be reverted. This patch fixes three bugs: 1. Memorize a function 2. Fix _get_crash_log() and 3. Remove an unnecessary import of a Python module.
Comment on attachment 301538 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301538&action=review > Tools/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=168329 Please add the Radar bug URL under this line before landing.
I will split this into the three requested bugs. This bug will continue to track the memoization change, I will include better descriptions of the rationale of the changes in the respective change logs.
Created attachment 301622 [details] Patch
Remove time import: https://bugs.webkit.org/show_bug.cgi?id=168371 Fix DarwinPort._get_crash_log(): https://bugs.webkit.org/show_bug.cgi?id=168372
Comment on attachment 301622 [details] Patch Clearing flags on attachment: 301622 Committed r212372: <http://trac.webkit.org/changeset/212372>
All reviewed patches have been landed. Closing bug.