appengine/findit/model/crash/cracas_crash_analysis.py - Issue 2322133002: [Findit] Add cracas_crash_analysis

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: appengine/findit/model/crash/cracas_crash_analysis.py

Issue 2322133002: [Findit] Add cracas_crash_analysis (Closed)

Patch Set: Overwrite _CreateKey method. Created 4 years, 3 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 # Copyright 2016 The Chromium Authors. All rights reserved.

	2 # Use of this source code is governed by a BSD-style license that can be

	3 # found in the LICENSE file.

	4

	5 import hashlib

	6 import json

	7

	8 from google.appengine.ext import ndb

	9

	10 from model.crash.fracas_crash_analysis import FracasCrashAnalysis

	11

	12 # TODO(katesonia): Deprecate FracasCrashAnalysis after switching from Fracas

	13 # to Cracas.

	14 class CracasCrashAnalysis(FracasCrashAnalysis):
	wrengr (wrong one) 2016/09/09 17:58:24 Like mbarbella says, this doesn't look clean. Espe Like mbarbella says, this doesn't look clean. Especially given the intent to deprecate FracasCrashAnalysis in the (presumably near) future, it'd be a lot better to just factor things out now. There should be some base class that contains the historic metadata and whatever else is shared, and then both the FracasCrashAnalysis and CracasCrashAnalysis will be thin subclasses that only override the method you mentioned. (Whether the shared base class is the same as CrashAnalysis or different is up to you.) Classes are cheap. Use them freely to help organize the code, name key concepts, and keep concerns separate. Even when there's only one small difference between a subclass and its superclass, if the classes are doing conceptually different things then make them different classes. The class hierarchy should match the conceptual ontology; since CracasCrashAnalysis isn't "a type of" FracasCrashAnalysis, it shouldn't inherit from there. Sharu Jiang 2016/09/09 23:19:42 Done. Show quoted text On 2016/09/09 17:58:24, wrengr (wrong one) wrote: > Like mbarbella says, this doesn't look clean. Especially given the intent to > deprecate FracasCrashAnalysis in the (presumably near) future, it'd be a lot > better to just factor things out now. There should be some base class that > contains the historic metadata and whatever else is shared, and then both the > FracasCrashAnalysis and CracasCrashAnalysis will be thin subclasses that only > override the method you mentioned. (Whether the shared base class is the same as > CrashAnalysis or different is up to you.) > > Classes are cheap. Use them freely to help organize the code, name key concepts, > and keep concerns separate. Even when there's only one small difference between > a subclass and its superclass, if the classes are doing conceptually different > things then make them different classes. The class hierarchy should match the > conceptual ontology; since CracasCrashAnalysis isn't "a type of" > FracasCrashAnalysis, it shouldn't inherit from there. Done.
	15 """Represents an analysis of a Chrome crash."""

	16

	17 @staticmethod

	18 def _CreateKey(crash_identifiers):

	19 return ndb.Key('CracasCrashAnalysis', hashlib.sha1(

	20 json.dumps(crash_identifiers, sort_keys=True)).hexdigest())

OLD	NEW

« no previous file with comments | « appengine/findit/crash/findit_for_client.py ('k') | no next file » | no next file with comments »