Page MenuHomePhabricator

print age warning if > 30 days old
AbandonedPublic

Authored by Firehed on Jan 12 2014, 9:10 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 19 2023, 2:17 AM
Unknown Object (File)
Nov 10 2023, 9:36 PM
Unknown Object (File)
Oct 17 2023, 1:44 PM
Unknown Object (File)
Aug 30 2023, 9:47 AM
Unknown Object (File)
Aug 26 2023, 5:34 PM
Unknown Object (File)
Aug 25 2023, 11:58 AM
Unknown Object (File)
Aug 13 2023, 2:40 AM
Unknown Object (File)
Aug 10 2023, 1:23 PM
Subscribers

Details

Reviewers
None
Group Reviewers
Blessed Reviewers
Summary

If arc itself is more than a month old, print a suggestion to the user to run 'arc upgrade'. This is inspired by composer.

This is a rough first pass that probably will not work on Windows and is overall somewhat clunky, but if you like the idea I am happy to improve it.

Test Plan

Changed 30 to 3 and 300. Made sure it printed or did not print appropriately.

Diff Detail

Branch
age_warning
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

I don't love this, conceptually:

  • Some installs run far behind HEAD (Facebook's install is currently circa early September).
  • The Conduit protocol itself is already versioned, and we issue instructions to upgrade when we hit a conduit version issue.
  • Some installs decouple arc maintenance from the user, and these instructions may not be good there.

In terms of this implementation specifically:

  • While unlikely, it is possible arc may not see changes for a month, which would make this impossible to dismiss.
  • This requires the .git metadata be retained (at Facebook, e.g., it is not maintained in production).
  • It requires git to be installed, which is not otherwise a requirement.
  • This unpredictably prints arbitrary data to stdout, which breaks scripting and --output json, etc.
  • And, yeah, it won't work on Windows.

Have you run into specific issues? Maybe we can attack those issues directly, like how we versioned the Conduit protocol. I'd ideally like to focus on more specific triggers for upgrade prompts, rather than correlated-but-nonspecific stuff like time or number of commits.

(If the issues you've hit are somewhat specific to your install, I think it's fairly straightforward to do this by overriding ArcanistConfiguration. I'd like to move away from that in the long run, but we could provide similar event hooks if we don't already have them.)

Totally fair response, hence a low-effort hacky first-pass :)

Some developers were having issues where arc diff wasn't able to figure out where a feature branch started, so we'd manually have to dig through git log to find the right commit and pass it as an argument into arc diff. This was only happening, as far as I could tell, when they pushed their feature branch upstream (see: Q20), either because it was long-running feature and we didn't want the scaryness of it living only on their machine or simply had multiple engineers working in parallel. I can't reproduce the issue for the life of me, which leads me to believe it's either a) fixed in a semi-recent version of arcanist or b) due to some bizarre workflow they shouldn't have been using in the first place.

Should it come up again, I have plans to get the relevant info and fix the actual issue, but that's been holding us up for a while to get everyone on the feature-as-commit workflow that arc diff encourages (and subsequently improving our deployment process, qa cycles, sprint planning, and a ton more). I'll file a bug and/or patch if I'm ever able to reproduce the thing, as it's my #1 concern.

Generally speaking I think that when you're not working off a tagged release it makes sense to stay close to HEAD of master, but that's just my opinion.