New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 661822 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 644372



Sign in to add a comment

[Findit] moar namedtuples! :)

Project Member Reported by wrengr@chromium.org, Nov 2 2016

Issue description

A bunch of our classes are just records/structs for passing a bundle of data around together, where the collection of fields is fixed, and where it doesn't make sense to mutate the values they comprise. These classes should be switched over to namedtuples. Using namedtuples helps ensure we don't accidentally mutate things, and greatly reduces the overhead of constructing the objects themselves.
 
Some examples include:
lib.gitiles.change_log.ChangeLog,
lib.gitiles.change_log.FileChangeInfo,
crash.changelist_classifier.ChangelistClassifier,
crash.results.Result.
Also the region_info in findit/util_scripts/git_checkout/local_git_parsers.py as of http://crrev.com/2435863003#730001

Comment 3 Deleted

Also maybe the commit_info (i.e., the values in that dict) for local_git_parsers.py? Though that one gets mutated a lot in the loop, so maybe it shouldn't be a namedtuple for the duration of the loop (though can be turned into one once we leave the loop).
Also the results of GetFileChangeInfo
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/infra.git/+/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386

commit 9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386
Author: wrengr <wrengr@chromium.org>
Date: Tue Nov 22 21:48:16 2016

Converting various classes to namedtuples

Also re-removing callstack.py, since a previous CL moved it into
stacktrace.py but somehow that got rebased improperly.

BUG= 661822 

Review-Url: https://codereview.chromium.org/2518663002

[delete] https://crrev.com/e5d7862c9beb05650a4405574c2cb28f33e87a3a/appengine/findit/crash/callstack.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/crash/changelist_classifier.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/crash/results.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/crash/stacktrace.py
[delete] https://crrev.com/e5d7862c9beb05650a4405574c2cb28f33e87a3a/appengine/findit/crash/test/callstack_test.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/crash/test/results_test.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/crash/test/stacktrace_test.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/lib/gitiles/change_log.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/util_scripts/git_checkout/local_git_parsers.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/util_scripts/git_checkout/test/local_git_parsers_test.py
[modify] https://crrev.com/9cacec9460f8fbcbb6cfe2f4bb4def59c3fc6386/appengine/findit/util_scripts/git_checkout/test/local_git_repository_test.py

Comment 7 by wrengr@chromium.org, Nov 22 2016

Status: Fixed (was: Assigned)
I think I got them all. If there are any other classes we should convert, then either reopen this ticket or make a new one.

Sign in to add a comment