Page MenuHomePhabricator

DiffusionCommitTagsHeraldField
AbandonedPublic

Authored by 20after4 on Jun 13 2016, 2:17 PM.
Tags
None
Referenced Files
F10899964: D16101.id.diff
Wed, Jul 6, 4:19 PM
Unknown Object (File)
Wed, Jul 6, 3:08 AM
Unknown Object (File)
Sun, Jul 3, 8:51 PM
Unknown Object (File)
Thu, Jun 30, 9:03 PM
Unknown Object (File)
Mon, Jun 27, 11:13 PM
Unknown Object (File)
Sat, Jun 25, 12:23 PM
Unknown Object (File)
Sat, Jun 25, 12:22 PM
Unknown Object (File)
Wed, Jun 22, 3:15 AM
Subscribers

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Summary

This is just like DiffusionCommitBranches but instead queries tags

Test Plan

untested

Diff Detail

Branch
wmf/stable
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 12613
Build 16014: arc lint + arc unit

Event Timeline

20after4 retitled this revision from to DiffusionCommitTagsHeraldField.
20after4 updated this object.
20after4 edited the test plan for this revision. (Show Details)
epriestley edited edge metadata.

This is fine technically, but I'm hesitant about introducing it because it makes the issues in T6522 worse in some sense. That is, this field really means something like:

Tags... that the commit is tagged with when it first appears in the repository

...which isn't very intuitive, and probably less intuitive with tags than with branches (you would expect tagging something with release-3 and then pushing the tag to trigger the rule, but it won't).

I don't remember the original discussion from IRC at this point, but I think it was something that might be resolved by forking (T10691)?

Generally, I'm not happy with where T6522 is right now and don't have a way forward on it yet. I'd like to have a clearer path forward before bringing fields like this upstream, at least without a specific use case which is strong enough to justify the greater peril from T6522.

This revision now requires changes to proceed.Jun 27 2016, 2:33 PM