Opened 2 years ago

Closed 2 years ago

#27180 closed enhancement (fixed)

Remove DetailsDocumentFields interface

Reported by: irl Owned by: irl
Priority: Medium Milestone:
Component: Metrics/Onionoo Version:
Severity: Normal Keywords:
Cc: metrics-team Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


This interface "Provides constants for details document field names".

In practice, it only provides 2 of them, and they're only used in one function that is full of other hard coded strings.

We can remove this to reduce complexity.

Child Tickets

Change History (7)

comment:1 Changed 2 years ago by irl

Owner: changed from metrics-team to irl
Status: newaccepted

comment:2 Changed 2 years ago by karsten

Or can we use it for the remaining fields, too? If not, I'm okay with removing this interface.

comment:3 Changed 2 years ago by irl

Status: acceptedneeds_review

Aaah, it is also used to build the OrderParameters. Now I'm unsure if we would want to remove this, or go the other direction and use it for all parameters in the documents.

Please review my commit 41c9a9b in my task/27180 branch to see what removing it looks like. Maybe this can help us decide which way to go.

comment:4 Changed 2 years ago by irl

Cc: metrics-team added

comment:5 Changed 2 years ago by karsten

I think it's okay to just remove it.

In fact, regarding OrderParameters, I think that these shouldn't use DetailsDocumentFields anyway, because parameter values and document fields are not necessarily the same thing. They happen to be the same in this case, and in most cases that makes sense, but there might be cases where they are not.

Should I merge this change then?

comment:6 Changed 2 years ago by irl

Yes. There are no user facing or operational changes so I don't think this needs a changelog entry.

comment:7 Changed 2 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

Merged. Closing. Thanks!

Note: See TracTickets for help on using tickets.