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

Issue 644476 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Last visit > 30 days ago
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 644372



Sign in to add a comment

[Findit/Predator] Better names for things

Project Member Reported by wrengr@chromium.org, Sep 6 2016

Issue description

Many of the classes have ambiguous or unhelpful names like "Result" or "Classifier" which don't give any real indication of what they are. (Most things can be "results", and we have many different forms of "classifiers" in Findit.) Some examples are:

* the Result class in infra/appengine/findit/crash/results.py. Should be renamed to something like "Suspect" or "Culprit". I (wrengr@) am still looking through the code to determine what the best name for this class would be.

* the MatchResults class in infra/appengine/findit/crash/results.py

* the Classifier class in infra/appengine/findit/crash/classifier.py (along with the FooClassifier subclasses). This class is for determining which large hunk of code contains the code responsible for some crash. At present we have two notions of "large hunk of code": projects, and components. It's unclear whether we'll ever have any other notions, but either way we need a better name that actually indicates what this class hierarchy is up to.

* the Stacktrace vs Callstack distinction is unfortunate. While both names make sense in a way, these two terms are generally synonymous, so we should make it clearer what the distinction is all about.
 
Also, in many places we pluralize things in non-standard ways (e.g., "infos", "indexes"). That should be cleaned up.

Comment 2 by st...@chromium.org, Oct 4 2016

Components: -Tools>Test>FindIt Tools>Test>FindIt>Crash

Comment 3 by st...@chromium.org, Oct 4 2016

Cc: kateso...@chromium.org st...@chromium.org
An example of overly generic names causing confusion:

CrashConfig.Get().component_classifier['top_n'] gives the number of stack frames to look at when trying to decide which component to blame.

CrashConfig.Get().fracas['top_n'] gives the number of stack frames to look at when trying to decide which CLs to blame.

While "top N" is correct for both, the names don't help much in figuring out how/why they're different. We should try to (a) give the fields more informative/complete names, or (b) rely on modularity to hide that sort of detail from functions that shouldn't care (e.g., findit_for_chromecrash.py), or (c) come up with a more fruitful generalization of classifiers (in the ML sense, not in the classifier.py sense) so that their relationship is more explicit.
Components: Tools>Test>Predator
Components: -Tools>Test>FindIt>Crash

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

Summary: [Findit/Predator] Better names for things (was: Better names for things in Findit)
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 19 2016

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

commit 0ff06cb8f8877d0296a2f387f443a782e28191d8
Author: wrengr <wrengr@chromium.org>
Date: Mon Dec 19 18:44:56 2016

[Predator] renamed "Result" to "Suspect"

BUG= 644476 
TBR=stgao@chromium.org

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

[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/changelist_classifier.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/component_classifier.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/culprit.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/loglinear/changelist_features/test/min_distance_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/loglinear/changelist_features/test/top_frame_index_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/loglinear/feature.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/project_classifier.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/scorers/min_distance.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/scorers/test/aggregated_scorer_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/scorers/test/aggregators_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/scorers/test/min_distance_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/scorers/test/top_frame_index_test.py
[rename] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/suspect.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/changelist_classifier_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/component_classifier_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/crash_pipeline_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/crash_test_suite.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/findit_for_chromecrash_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/occurrence_test.py
[modify] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/project_classifier_test.py
[rename] https://crrev.com/0ff06cb8f8877d0296a2f387f443a782e28191d8/appengine/findit/crash/test/suspect_test.py

Status: Archived (was: Assigned)

Sign in to add a comment