1633660 - Attribute inconsistency in alert summary serializer

Status: NEW

(Tree Management :: Perfherder, enhancement, P3)

Description

[Ionuț Goldan [:igoldan]]

While reviewing what caused the framework id/name confusion bug 1632074, I noticed we have an attribute inconsistency in the PerformanceAlertSummarySerializer class.

This class has 2 attributes: repository & framework. First one is a name, second one is an database id. This isn't alright & confusing. Either both are names or both are ids or both encapsulate the id & name.

Comment 1

[Alexandru Ionescu (needinfo me) [:alexandrui]]

Note that if we go for both names without updating templateArgs.framework will regress the regression template and we won't be able to file any regression until this is fixed.

Comment 2

[Kimberly Sereduck :kimberlythegeek]

preference on whether to use id or name?

Comment 3

[Alexandru Ionescu (needinfo me) [:alexandrui]]

I would leave both as the current attributes are used in a bunch of places and leaving just one of id or name is a refactoring that doesn't worth IMHO. I would just rename repository to repoName, framework to frameworkId and add the other two.

Comment 4

[Kimberly Sereduck :kimberlythegeek]

(In reply to Alexandru Ionescu (needinfo me) :alexandrui from comment #3)

I would leave both as the current attributes are used in a bunch of places and leaving just one of id or name is a refactoring that doesn't worth IMHO. I would just rename repository to repoName, framework to frameworkId and add the other two.

Sounds good to me! Thanks!