In T8238#196204, @epriestley wrote:

I'm a little confused about how these tags are appearing so widely in developer working copies in the first place. Is git clone fetching them by default? Do you have unusual default options configured? Do users regularly explicitly fetch all tags present in the remote?

That is, my expectation is that git pull does not "bring everything back".

In this specific case (staging same as origin) we could just not push the base tag (it has no value in this case). We can probably even figure that out automatically without needing configuration. That would be a cleaner fix, I think?

I've been using this functionality for some days to use the CircleCI integration. Mostly the user experience is fine. It's mostly possible to configure git log to ignore these tags, like:

Latest version seems to have fixed this. Sorry for the noise, and thanks for building such an awesome software.

Doh. Give me a minute, testing with 3493d9d5138e0a630624bced548090163ce9be8a now.

That's a good point, and certainly makes me feel less bad about putting IAM user credentials in a file on the host.

I think you missed the point. The attacker in this instance is an employee, or a contractor, or an ex-employee. If you have static credentials sitting in files on your servers, all your ex-employees probably have them, too. And they can use them until you rotate them, which is pretty much never, in practice.

OK, fair enough. Thank you for clarifying.

I'm confused. If by "live" credentials you mean credentials that exist in some AWS account and grant some level of access to S3, I'm not sure what other kind of credentials I'd ever use.

For me, the big advantage to using IAM roles is that the credentials rotate automatically, every hour.

I think the real problem is right here:

I think you are misunderstanding my proposal.

My take on T5378 is that links don't get the same helpful context as tokens, such as:

I've got to rebump this issue. I've started a new job since I last commented, and the current review process needs some work. Currently we are using GitHub PRs for review and I don't like it. I've installed Phabricator for evaluation, and so far both of two people who have tried it have made this mistake.

• make option name consistent with other options

OK looks like you got it, but I was able to reproduce here at https://secure.phabricator.com/D8994#23. Like you say, seems you must have inline comments without a normal comment.

OK...looks like this instance is broken in a slightly different way. Here's what I got when I tried to edit:

Actually, I forgot a key part of what I wrote in my reproduction steps: edit that comment. When I look at D8994, there's no option to edit. On my instance, it seems like I can go back to any revision, edit an inline comment, and it disappears. Why can I edit on mine but not yours?

Still need to try updating my version. Perhaps a significant difference: I updated the diffs here "via web". On my instance it was done with arc, and it says "via conduit".

Modifying README is so 2000. We should modify LICENSE instead, because that has dire legal consequences.

Be polite.

Reloading does not fix it. I tried to reproduce at D8994, but was unable. I'll try updating my install and see if that changes anything.

I guess this wasn't a good idea, anyway.

Test emphatically, but just once.

Test more emphatically.

Probably if there is a thing being blamed, it should be commits, not revisions. It may be that a commit was made without review that broke something, but never will it be the case that an un-committed revision broke anything. Also if you subscribe to DaggyFixes, then blaming a commit makes it clear from where the fix should be based. The workflow might look something like this:

Related to this are commits that aren't reverts, but are fixes to regressions. Maybe whatever problem was initially introduced wasn't so horrible that it first had to be reverted -- it was easy enough to just fix it. A fix might link back to a revision that introduced the error. Addressing concerns from audits is related: if I author a revision to address an audit concern, it might be nice to formally link the two.

Totally agree with not having separate "importance" and "urgency" fields -- that is too much process to exist in the product and force upon everyone. The biggest problem I'd like to see solved here is just the surprise I get when dragging things around in the workboard. That could be fixed either by separating ordering from priority, or making the current behavior more clear, UX-wise.

More practically, we can punt on solving this, resolve ourselves to not being able to upload files (other than the drag and drop upload), and just cut the 'attach file' feature from Maniphest altogether.

Fixes the problem and doesn't break things for me now. Maybe you want an actual developer like @epriestley to review it also -- I really have no idea why what you changed makes a difference.

@chad, I'm not proposing auto-save. I'm saying leave everything as it is, and get rid of the button that does nothing.

@btrahan, I'm setting "Action: Upload File" in the comment box and attaching the file, but it's not showing up. Oddly, the preview says "bitglue changed files, attached: ; detached: ". Guess that's a separate problem. Anyhow, here's the arc output and apache log: https://gist.github.com/bitglue/8c6f53331186577c4bfc

Also apache error.log attached.

With D8757 applied, I can't arc diff new changes. A log of output with --trace is attached.

This breaks arc diff for me. I think I can attach a log to T4774...

If there's some technical reason getting rid of inline comment buttons is hard, then I propose that the button says "preview", and that it's not blue. I think that's still confusing, but at least people will be confused immediately and directly, rather than thinking everything went fine then confused when other people can't see their comments.

@chad, why do you need a button at all? Not-inline comments have just two states:

If I can weigh in with my own opinion: the workflow is good as it is -- it's just not obvious what that workflow is. I posit that the blue "ready" button serves no purpose except to confuse. I don't know why you'd need to "ready" a comment. You've typed it in, and it's ready. It doesn't help that there are big blue buttons everywhere in Phabricator with whimsical titles. I like the whimsy, and in all cases but this one it's fine, because these buttons mean one thing: "do the thing". But "ready" doesn't do a thing -- all it does is animate the box to make it look like the thing was done. No one bothers to read "Not Yet Submitted", because they are already convinced, by having clicked the big blue button, and having seen the animation, that the thing they wanted to do has been done.

A rather poor workaround is:

Use case for me is creating a herald rule to match revisions which were merged without ever being reviewed so they can instead be audits. Sometimes no one is available to do reviews and I need to more forward so I give up waiting and merge without review, but I still want someone to look at the commit eventually.

I can confirm this issue. I have about 3500 commits and 170 branches, and was running out of memory on a 1GB VM.

OMG EMAIL COMMENT!

Sounds like a good plan.