Open Bug 1633660 Opened 1 year ago Updated 1 year ago

Attribute inconsistency in alert summary serializer

Categories

(Tree Management :: Perfherder, enhancement, P3)

enhancement

Tracking

(Not tracked)

People

(Reporter: igoldan, Unassigned)

References

Details

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.

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.

preference on whether to use id or name?

Flags: needinfo?(igoldan)
Flags: needinfo?(aionescu)

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.

Flags: needinfo?(aionescu) → needinfo?(ksereduck)

(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!

Flags: needinfo?(ksereduck)
Flags: needinfo?(igoldan)
You need to log in before you can comment on or make changes to this bug.