add re-reviews to the MIR process #17
Conversation
In the last sprint a thought that has come up is the general desire to take our mission of "quality in main" serious and that actually re-reviews of packages already in main would need to be part of that. This starts to document what we could do to start the discussion as only about half the team was in the in-person session. The rules are 60% subset of the iniital MIR rules and 10% slight modifications or small additions. It might make more sense to not create a second redundant template and instead somehow "mark" those that shall be "also done" or "only done" on re-reviews in the original template. Open for suggestions, as I said this is only meant to be a draft to get the ball rolling. Signed-off-by: Christian Ehrhardt <[email protected]>
cpaelzer
requested review from
slyon,
didrocks,
setharnold,
javacruft,
joalif,
eslerm and
dviererbe
There was a problem hiding this comment.
Overall I'm a +1 for the re-review process.
I believe we will find a few packages missing tests.
I think we should discuss/decide how strict (or not) the rules shall be in that case, given
that the owning teams may not have the bandwidth to implement test suites.
There was a problem hiding this comment.
praise: +1 for me, I really like the the Idea to re-evaluate the quality of packages in main (especially the old ones) so we can take action. Thanks for proposing this!
question: Is this a one-time effort, or should this evolve into a continual practice to regularly re-evaluate packages in main?
There was a problem hiding this comment.
praise: +1 for me, I really like the the Idea to re-evaluate the quality of packages in main (especially the old ones) so we can take action. Thanks for proposing this!
question: Is this a one-time effort, or should this evolve into a continual practice to regularly re-evaluate packages in main?
This should become a continued effort. Let's say we look at packages/MIRs which haven't been touched (from a MIR perspective) in 5 years. There might be a time (in the distant future) where we are so much on top of it that there's nothing left to re-review. But the rolling 5 year window will make new re-review trickle in over time endlessly. |
|
++1 I really like this suggestion. It will create more initial work, but the net-effect will be an easier to maintain and more secure distro. This needs spellcheck linting. How are conditional ACKs handled? What leverage does the MIR team have, if conditions need to be met for a package to remain in main. e.g., what happens when ACK requirements have not been met a year after the review. |
There was a problem hiding this comment.
[once current typo/fixes are managed]
I really like it in general. I have slight concerns about the managements ack as some of us are not fully focused on distro work, but rather seen as best effort.
If the 1 per week covers either old MIR review OR new MIR, I think this is reasonable though.
Thanks, I think we want to push, but not too hard. |
@dviererbe it will be continuous, I added wording to explain |
Thanks @eslerm - I have put some words to outline my expectation in regard to this into the new version. |
Based on the great initial feedback this improves the PR by: - Fixing various typos - Adding improved wording in various places - Defining a few aspects that were formerly unclear or missing - Restructuring the rereview template to be part of the normal template avoiding redundancy Signed-off-by: Christian Ehrhardt <[email protected]>
|
FYI: I pushed a new version which I hope addresses all of the feedback and discussion we had so far. Maybe we can again get reviews (I marked all of you for review again) before next MIR meeting. |
Based on further feedback: - add explanations where incomplete or misleading - fix a few typos Signed-off-by: Christian Ehrhardt <[email protected]>
|
Hey MIR team. I ended up here after reading yesterday's IRC meeting's log, unsure if there is a more proper place to have that discussion but to me it feels like we should have Sharing my personal initial thinking. While I understand the intend, it seems a ton of work that is going to be invested for really little benefit. Speaking for desktop we already struggle to find the resources to work on the package improvements needed to MIR the new packages we need promote, there is no way we will be able to take on extra tasks with our current resourcing. I'm not going to start a discussion on the interest of debian/copyright, but even if we find resources to spend on improving our packages quality (which I think we should, at least for example on automated testing), updating debian/copyright is bring no value to our users and our product and I don't see us wanting to allocate resources to such tasks when we have and endless backlog of users visible issues/QA/workflow improvements to work on. If you want to go ahead with that effort my preference would be to at least try to narrow a lot the checklist to focus on the things that will bring the most value back (build and autopkg tests, security review, ...) I would be interested to see what other teams that will be on the 'receiving end' think about the proposal. |
We, as a company, need an SBOM solution soon to sell to some customers. If debian/copyright files are consulted by the SBOM tools under consideration, we might need to get these into good shape very quickly. If debian/copyright files aren't used by the SBOM tools under consideration, then any time spent on them feels 100% wasted. (At least, I've never seen any praise for them from my Debian circles, just complaints about how complex they are to prepare. Perhaps they do serve a useful purpose but it's well-hidden if so.) |
| 1. But on the other hand this keeps effort of MIR team members somewhat plannable | ||
| 1. We'd run a trimmed down check which is a subset of our usual check (see below for details) | ||
| 1. Anything we find for packages already in main becomes part of the owning teams backlog. The decision how hard we push on them is a case by case discussion depending on too many variables to properly pre-define. | ||
| 1. As on the initial MIR review we might end up recommending a security (re-)review which goes into their queue at low prio |
There was a problem hiding this comment.
please change "we might end up recommending a security (re-)review" to "we might assign a security (re-)review"
I also recommend removing "which goes into their queue at low prio". Priority of a Security MIR is set by the owning team relative to the owning teams other open security MIRs. We try to work on MIRs across all teams evenly, and having owning team set priority helps us know what is most relevant to them. It is exceedingly likely that an owning team will de-prioritize re-reviews though :)
|
|
||
| The proposed approach would be to: | ||
|
|
||
| 1. We will add (doing so is a TODO) a query that helps us to spot packages in main for the longest time without having a recent update on their MIR bugs. |
There was a problem hiding this comment.
Could re-review be avoided by comment bumping an MIR bug?
Re-assessing oldest MIRs first sounds fair, but may not be the most relevant. I don't have a better solution.
| @@ -850,6 +931,12 @@ TODO-B: - translation present | |||
| TODO-A: Problems: | |||
| TODO-A: - TBD | |||
| TODO-B: Problems: None | |||
|
|
|||
| [Re-Review*] | |||
There was a problem hiding this comment.
Should there be a requirement to see if upstream code is still maintained? e.g., that a git repo is not in archive mode
Perhaps this isn't Re-Review specific.
|
Leaving some comments for the current state of this:
Until we have a either resources or at least a system to detect either "major change, worth to check" or "super old case, surely worth a look" this will be blocked in the interim state. |
In the last sprint a thought that has come up is the general desire to take our mission of "quality in main" serious and that actually re-reviews of packages already in main would need to be part of that.
This starts to document what we could do to start the discussion as only about half the team was in the in-person session.
The rules are 60% subset of the iniital MIR rules and 10% slight modifications or small additions. It might make more sense to not create a second redundant template and instead somehow "mark" those that shall be "also done" or "only done" on re-reviews in the original template. Open for suggestions, as I said this is only meant to be a draft to get the ball rolling.