Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean-up in database configuration #1165

Open
matsduf opened this issue Apr 26, 2024 · 5 comments
Open

Clean-up in database configuration #1165

matsduf opened this issue Apr 26, 2024 · 5 comments
Milestone

Comments

@matsduf
Copy link
Contributor

matsduf commented Apr 26, 2024

As far as I can see Zonemaster::Backend::DB::sub store_results() is not used anywhere besides in t/lifecycle.tand could be removed.

https://github.com/zonemaster/zonemaster-backend/blob/develop/lib/Zonemaster/Backend/DB.pm#L431-L449

Furthermore, the database definitions of table "test_results" include a field "results" which is not used anymore, as far as I can see.

@matsduf matsduf added this to the v2024.1 milestone Apr 26, 2024
@marc-vanderwal
Copy link
Contributor

Zonemaster::Backend::DB::store_results seems to be used only in t/lifecycle.t. This unit test file isn’t trivial and needs careful scrutiny before deleting anything.

Before dropping the results column from the test_results table, some queries need to be rewritten in Zonemaster::Backend::DB. One offender is in the store_results function (and might just be dead code) and two more are in select_unfinished_tests.

@anandb-ripencc
Copy link
Contributor

This issue was meant to be resolved for 2024.1. Has it been resolved, or did it not make it to the 2024.1 version?

@tgreenx
Copy link
Contributor

tgreenx commented Jul 3, 2024

This issue was meant to be resolved for 2024.1. Has it been resolved, or did it not make it to the 2024.1 version?

@anandb-ripencc Unfortunately it did not get fixed yet. Is this a priority on your end?

@anandb-ripencc
Copy link
Contributor

It's not a priority, because the obsolete "results" column doesn't do any harm. I was just wondering about the status, because this was marked for fixing in 2024.1.

@tgreenx
Copy link
Contributor

tgreenx commented Jul 3, 2024

It's not a priority, because the obsolete "results" column doesn't do any harm. I was just wondering about the status, because this was marked for fixing in 2024.1.

I see. Honestly our current way of assigning milestones on issues is mostly untrustworthy. Usually most issues are assigned the milestone of the next-to-come release, unless we have good reason to do otherwise. A better indicator is to look at issues marked as high priority ("P-High") and/or that come directly from users ("T-Question").

@tgreenx tgreenx modified the milestones: v2024.1, v2024.2 Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants