Page MenuHomePhabricator

ReleaseTaggerBot assumes that a task can only be tagged with one release per minor version?
Open, Needs TriagePublic

Description

I'm not sure what the logic is, or whether this is intentional, but I noticed that we might be removing tags incorrectly. In task T228851, I see several retagging actions which are probably related to different patches. At one point, I tried to fool the bot by deploying a patch via a SWAT and the bot very impressively removed the expected, scheduled deployment tag, adding the earlier version. However, when we linked an additional patch to the task, the bot removed the earlier tag and added the later tag, again.

I think the correct behavior is to reprocess all linked patches, each time there is a change which might affect where they might be deployed. The tags should be unioned, and then any stale tags removed.

Finally: thank you for this helpful tool!

Event Timeline

This is an (intentional) side-effect of Phabricator's milestone concept.

This is an (intentional) side-effect of Phabricator's milestone concept.

I'm happy close this task if the feature as described makes sense to others, but to my limited understanding it feels arbitrary, and I'd like to be able to trust the release tags to show me which fixes and features will show up in each release.

In Phabricator, "Tasks in Milestones can only be in one Milestone."
Nothing that could be fixed in ReleaseTaggerBot code, hence proposing to close this task as invalid.

If you think that Phabricator upstream code should be adjusted, feel free to bring this up in https://discourse.phabricator-community.org/

The RTB code could theoretically be changed from 'last patch merged' to 'lowest version number', or something along those lines -- but this would be a complex bit of code: currently, we add a hashtag based on the branch, and phabricator will resolve this to a tag through an alias. To support 'lowest version number', we'd need to parse all tags and somehow figure out which version they are.

Changing to 'first patch merged' (i.e. 'do not add a milestone if there is already a milestone') would be simpler, but I'm not sure if that behavior makes more sense than the current one.