Page MenuHomePhabricator
Create Task
Maniphest T12722

Auto-resolve with "Fix: T..." commit message should not reopen a closed bug
Closed, InvalidPublic
Actions

Description

Here is our use case:

We have three bug status: "Open", "Resolved", "Closed"
And there are two branches that we actively work with, "master" and "release".

Consider the following work flow:

  1. Someone find a bug with "release" branch and open a maniphest ticket, let's say T1
  2. someone check in a fix with commit msg indicating "Fix T..." that automatically marks T1 as "Resolved"
  3. QA team verifies the fix in "release" branch and mark the bug T1 as "Closed" (this happens before the fix is merged back to "master" branch, because we want to make sure everything is closed in release branch before release)
  4. later when fix is merged back to "master", since the merge commit message contains "Fix T...", it will then reopen the ticket by changing its status from "Closed" to "Resolved"

I understand phabricator could be configured such that auto-close through commit only applies to one branch, but in this case we do want it to work for both "master" and "release" branches; is it possible to only apply "auto-resolve" for open bug but do not reopen a closed bug?

Phabricator version: (stable) Promote 2017 Week 2
This is consistently reproducible

Event Timeline

zhiyuchen created this task.May 17 2017, 6:34 PM
zhiyuchen updated the task description. (Show Details)
chad added a subscriber: chad.May 17 2017, 6:42 PM

Bug reports require version information, and repeatable, detailed reproduction information. Please!

chad added a comment.May 17 2017, 6:44 PM

(Offhand this sounds expected)

zhiyuchen updated the task description. (Show Details)May 17 2017, 6:56 PM
zhiyuchen updated the task description. (Show Details)
avivey edited projects, added Bug Report (Needs Information); removed Bug Report.May 18 2017, 5:09 PM
zhiyuchen added a subscriber: avivey.May 18 2017, 6:50 PM

@avivey I updated description to include version info, repeatable and steps to reproduce, is there anything else you guys need?

avivey added a comment.May 18 2017, 7:30 PM

See Providing Reproduction Steps.

We need to have enough information to actually see the problem happening, so we can figure out what's wrong, and maybe even test a fix.

We also need fuller version information (All commit hashes), not just a title. And you need to be up-to-date; "2017 week 2" is about 4 months out.

zhiyuchen added a comment.May 18 2017, 9:56 PM

image.png (784×1 px, 83 KB)

@avivey If you follow the steps in my description, you should be able to reproduce the problem. I'm also attaching a screenshot which indicates that a maniphest was reopened from "Accepted" to "Resolved" twice because the merge commit msg also contains the original commit msg "... fix T...."

I will upgrade my phabricator install soon and try this again.

zhiyuchen updated the task description. (Show Details)May 18 2017, 9:57 PM
chad added a comment.May 19 2017, 12:15 AM

It still doesn't sound like a bug to me, or maybe more specifically, understand what you expect the upstream to fix.

chad added a comment.May 19 2017, 12:19 AM

Basically, you're choosing to use a singular task status to track the various lifetime stages of a bug through multiple branches, which isn't what we intended that field for. It's just as easy to expect QA to want to re-confirm the bug is fixed in master as it is your workflow. I think you should track QA in a custom field.

epriestley added a subscriber: epriestley.May 19 2017, 12:20 AM

Currently, Fixes T123 in a commit message means this:

Change the status of T123 to the configured default "closed" status.

I think @zhiyuchen would like Fixes T123 in a commit message to mean this:

If T123 is not already in a "closed" status, change the status of T123 to the configured default "closed" status.

The current behavior of Fixes T123 is intentional, and this is not a bug.

joshuaspence added a subscriber: joshuaspence.May 23 2017, 2:25 PM
epriestley closed this task as Invalid.May 30 2017, 6:08 PM

This doesn't appear to describe a bug in Phabricator.

Phabricator · Built by Phacility