Opened 9 months ago

Closed 9 months ago

#27234 closed task (fixed)

Clean up and gently refactor metrics codebases

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

Description

This week seems like a good time to finally clean up and gently refactor the metrics codebases. There are currently no large, unmerged branches open, and we did not schedule any releases for this week.

I started making changes to all five codebases. Most of the changes are based on warnings produced by IntelliJ's Code Analysis tool. None of the changes are meant to change behavior.

Reviews will be a challenge, which is why I'm trying to do only one type of change per commit. Also, I'm pushing commits as early as I can while continuing to make more changes. I'd appreciate a 2-step review process where the first step is to very quickly decide whether a commit is useful or maybe even harmful and the second step is to review in detail whether changes in a commit are correct.

I'll post 5 branches as soon as I have a ticket number. I'll also comment whenever I post new commits to any of these branches.

Setting priority to high, because we should finish this work before resuming any other tasks.

Child Tickets

TicketStatusOwnerSummaryComponent
#26190closedmetrics-teamUse slf4j throughout all stats modulesMetrics/Statistics

Change History (9)

comment:2 Changed 9 months ago by karsten

Just pushed another bunch of commits to the branches above.

comment:3 Changed 9 months ago by karsten

Pushed another commit to the metrics-web branch.

comment:4 Changed 9 months ago by irl

Reviewer: irl

Going to go through these now one codebase at a time, just finished with CollecTor and that one is looking good to merge.

comment:5 Changed 9 months ago by irl

Ah, looking at the JavaDoc changes, possible thing to do differently:

Do we need to have the </p> tags on the ends of paragraphs? I thought that JavaDoc coped fine with just the opening tags and figured out the </p> tags on its own. It makes the comment a bit more readable to not have so much markup in it.

Not a huge thing, but just wondering.

comment:6 Changed 9 months ago by irl

Other than above, metrics-lib looks good.

comment:7 Changed 9 months ago by irl

Status: needs_reviewmerge_ready

Onionoo, ExoneraTor and metrics-web are also looking good. I think it is fine to merge these changes.

comment:8 Changed 9 months ago by karsten

Thanks a lot for the review! I'm sure it wasn't as much fun. Yet it's important to finally make all these changes.

Regarding </p> tags, I didn't know that they are optional. But I'm also not sure whether leaving them out is actually an improvement over including them. Maybe we can discuss that in the context of writing more/better JavaDocs for the five codebases.

I'll merge all five branches now, so that we can make unrelated changes to codebases again. No need to "lock" them any longer than necessary.

comment:9 Changed 9 months ago by karsten

Resolution: fixed
Status: merge_readyclosed

Merged all five branches! Closing. Thanks again!

Note: See TracTickets for help on using tickets.