+1 for bringing this back.
- Queries
- All Stories
- Search
- Advanced Search
- Transactions
- Transaction Logs
All Stories
Jun 15 2017
Thanks! Will check.
Jun 14 2017
I think this is worth acting on (by changing MySQL sandboxing or config sandboxing or some combination of things) if:
Oh, I don't think anyone should ever be running unit tests with actual production credentials. You should assume that arc unit will immediately escape all sandboxing and destroy all the data it can touch, then run BENCHMARK() for an hour and then crash the server. But you should assume arc unit will do this even if it is "supposed" to be Totally Seriously Sandboxed 100% into SQLite.
I'm not totally sure about how/why users are expecting the behavior in the third case.
@aurelijus, D18111 is landed now in HEAD if you'd like to test this. Let us know if you have any issues.
Philosophically I'm happy with that. Just as an exercise though, I came up with:
I'm not particularly strongly opposed to adding more options here, I just want to avoid the dropdown becoming this:
For the compliance case, you should already be able to trigger this stronger rule ("always add the package as a reviewer, even if an owner authored it") with Herald, by writing this rule:
As more of a point of philosophy of code review also, I (and, I think, my teammates) would say that diffs should be reviewed by someone who both:
- Understands the relevant code
- Did not write the code themselves
In other words, the combination of expertise + different perspective is of value beyond each component individually. If you believe that, then B or C should review A's diff in this case.
I can't really come up with a problem that would prevent, even theoretically -- do you have a specific scenario in mind offhand where some kind of MySQL configuration could mask a test failure locally?
When you put it that way, it does seem backwards.
That makes sense. Probably not much to be done for it (other than potentially doing a config override for maniphest.priorities prior to running ManiphestTaskTestCase).
This is semi-intended, because we have to pull a lot of config (like mysql.host) from your local install for things to make sense: we can't create test fixtures if we don't know how to connect to the database.
The use case in this task's description does not make sense to me.
Oh, this doesn't isolate things because they're on different databases, and thus we establish different connections. The daemon insert does not happen inside a transaction.
We're interested in this option, since we find that users are often surprised by the existing behaviour. I think this would make sense as a per-package setting, rather than a global one.
I have spent a little time poking at code, trying to figure out how to build a simple template engine into the description field, so that you can reference hidden custom fields by {field-identifier} or similar from within the remarkup. I resigned myself to building something just like publish.php, where the descriptions are maintained via conduit, however, I still long for it to be built in.
It would be really nice to default newly added fields to hidden. Going through 20+ forms to hide the fields is tedious.
We actually do this ourselves in the "Locations" technical interview scenario: the script tries to create a set of tasks, or update them if they already exist. It currently uses title-based search to accomplish this, but could more reliably use "utility labels" if they existed.
We do something similar ourselves for technical interviews, but I just related all the tasks by embedding them in the description of a central task:
I think this use case is a reasonable one.
Just discovered I could use the Milestones feature to do that. Will try this out.
Jun 13 2017
Yeah -- the workflow from the Maniphest side dumps all the values into inputs in the body and does a POST, I believe.
Interestingly I worked around the issue by doing the batch edit from the Maniphest search UI which is passing the selected tasks an HTTP body instead of a URL query I guess?
@swisspol hit a variant of this with 1,172 tasks bumping up against an Apache URL limit in the Phacility cluster when editing from a workboard column.
(Fine to keep open so I don't forget)
There will be a place to get to this.
(I've extracted the graph issue to T12840)
Should not, but looks like there are. I updated to master now, and it works now.
I used the current version, but there should be no meaningful changes to this behavior between your version and HEAD.
That's weird since I exactly did the same with my version from above. Did you used the current or my version? Or should I update and try if I can reproduce it at the master?
I can't reproduce this. Here's what I did:
Thanks :)
Version infos are:
phabricator
0d1654446be6de43b2bd12fcc247db0dc1a46464 (Sat, Jun 10)
arcanist
c04f141ab0231e593a513356b3832a30f9404627 (Fri, Jun 9)
phutil
74a1350416eb2df825c2315d6519bee03f77bee9 (Mon, Jun 5)In T12804#227133, @nickhutchinson wrote:Equally valuable as "what branches contain that commit" is "what commits are in the branches we care about". For us, the answer to this is the commit graph -- it's a succinct, easy to understand representation of our branches and their relationships.
I get that if you only have one primary branch, there's not much value in the commit graph. Similarly, if you're not linearising history, your commit graph will be so messy as to be meaningless. But if you do have the requirement to actively maintain several releases of some B2B software package and have a well defined branching strategy for your team to make this happen, then the commit graph is meaningful and unexpected merges will stick out.
Thanks! We've deployed this and are soliciting select feedback. Will get back to you shortly.
How did you generate this diff?
For perspective we use commit hooks to ensure that commits made in patch releases must exist in upstream major release first before being allowed in the respective patch branch. We then utilize herald to notify a group about suspicious merges or commits.
In T12804#227102, @avivey wrote:I think we have it documented somewhere that "we have developers who refuse to learn git" is considered a bad reason to ask for a feature, and you should maybe consider solving this issue with some workflow changes.
Note also that in your link, epriestley saysIn projects which do not linearize history with arc land + squash merges, I believe this feature is virtually useless because many commits are merges and visualizing the repository isn't very informative (the history has so many parent/child relationships that none of them convey much of anything).
which can translate to "We don't understand exactly what you're trying to see - what's the root issue?"
From your description, it sounds like a listing of "is commit in branch" for a given commit and all/some branches would be more useful for you.
Set new action
I think we have it documented somewhere that "we have developers who refuse to learn git" is considered a bad reason to ask for a feature, and you should maybe consider solving this issue with some workflow changes.
Note also that in your link, epriestley says
In projects which do not linearize history with arc land + squash merges, I believe this feature is virtually useless because many commits are merges and visualizing the repository isn't very informative (the history has so many parent/child relationships that none of them convey much of anything).
I'd like the commit graph to come back in some form -- looks like it was removed in rPc5bb69fd7d79. For the reasons @epriestley outlined here: rPf2fcafb40dde#38208, this view is really useful for visualising the relationship between Git branches and sanity-checking recent merge commits. These are tasks that would otherwise require something like gitk.
Thanks @epriestley, that fixed it indeed!
Jun 12 2017
Two questions pick at me:
- Which editor did the user use to add the ZWS in? Any editor I remember does something to warn you about it.
- Why are there Unicode characters for "goat" (🐐) and for "cooked rice" (🍚)?
T929 is probably the earliest request for this feature :)
I moved everything except one test instance off in T12817, and we appear to be ready for this host to die.
This has landed for anyone who wants to provide feedback on the v0 version.
I believe this should now be resolved in HEAD of master. Let us know if it doesn't work for you or if you're still seeing issues after upgrading. Thanks for the report!
No one has complained that all their data is gone so I'm going to assume the best here.