Implement analysis-related behavior for moderated package versions. #7639
Conversation
| // Make changes! | ||
| state.versions! | ||
| // Remove versions that have been deselected - but keep the latest finished one | ||
| ..removeWhere((v, _) => | ||
| deselectedVersions.contains(v) && | ||
| v != latestFinishedVersion.toString()) | ||
| // Add versions we should be tracking | ||
| ..addAll({ | ||
| for (final v in untrackedVersions) | ||
| v: PackageVersionStateInfo( | ||
| scheduled: initialTimestamp, | ||
| attempts: 0, | ||
| ), | ||
| }); | ||
| for (final v in deselectedVersions) { | ||
| // Remove versions that have been deselected, but: | ||
| // - keep the latest finished one, unless it is moderated | ||
| final pv = packageVersions.firstWhereOrNull((pv) => pv.version == v); | ||
| final remove = pv == null || | ||
| pv.isModerated || | ||
| v != latestFinishedVersion.toString(); | ||
| if (remove) { | ||
| state.versions!.remove(v); | ||
| } | ||
| } | ||
| // Add versions we should be tracking | ||
| for (final v in untrackedVersions) { | ||
| state.versions![v] = PackageVersionStateInfo( | ||
| scheduled: initialTimestamp, | ||
| attempts: 0, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Let's take this back a few iterations. Sorry, if this expands scope a bit.
I think the changes introduced in #7155 make the system hard to reason about.
_versionsToTrack is:
- a function of the
packageandpackageVersions. - It decides what versions are tracked and what versions are not tracked.
- It does not depend on
PackageStateand shall not.
What versions are tracked can be intuitively derived from package and packageVersions.
The // Make changes! section is supposed to make changes that were previously decided.
The // Make changes! section is not supposed to make new decisions based on additional state we didn't have when we made the previous decisions. This makes it hard to reason about why which decisions was made.
In #7155 we hacked // Make changes! to use PackageState in making decisions. This is problematic because:
- Decisions were already made, the comment "Make changes" implies that we are making changes based on decisions previously made. That became a lie.
PackageStateis not passed to_versionsToTrack, so it can't be part of the decision of what versions to track.PackageStateshould not be used to decide what versions to track, this makes it impossible to predict which versions will be tracked.
I suppose the goal was to avoid untracking the latest version that had analysis results, because that meant that we had no analysis the second a new version is published.
I can see how having no analysis for a while is sad. I would pick it over hard to predict behavior any day.
But we can also make predictable behavior that is likely to get us a similar result.
We could augment _versionsToTrack also track the latest 3 stable versions.
example:
2.3.0tracked, because it's the latest stable release.2.2.2tracked, because it's in the latest 3 stable versions.2.2.1tracked, because it's in the latest 3 stable versions.2.2.0no tracked.2.1.0no tracked.2.0.0no tracked.1.3.0tracked, because it's in the 5 latest major versions1.2.0no tracked.1.1.0no tracked.1.0.0no tracked.
We could come with other rules that does the same thing.
But fundamentally, we can come with a rule such that the previous stable version doesn't necessarily become untracked as soon as a new stable version is published.
We don't have to say: "track the latest 3 stable versions."
We could just say: "track the latest 3 versions".
Then we hope that we can analyze the new version before 3 new versions have been published, such that the new version won't be tracked anymore.
I think that's fairly likely.
(doesn't even have to be 3, I think 2 would do)
Yes, it wouldn't cover all cases as well as using PackageState to determine what to track, but we'd keep predictable behavior, and avoid the issue of having no analysis / dartdoc in most cases.
Notably, if new analysis starts failing, we'll keep reanalyzing an old version because it never disappears from tracked versions. That's weird, and hard to explain. Indeed we can't even know if we have a bug, or if we had a weird scenario where an old version was kept tracked because all newer versions consistently failed.
There was a problem hiding this comment.
Agreed, and submitting it as a separate PR.
#7535