drake icon indicating copy to clipboard operation
drake copied to clipboard

add some HPolyhedron utility methods

Open AlexandreAmice opened this issue 2 years ago • 8 comments

This change is Reviewable

AlexandreAmice avatar Apr 29 '22 14:04 AlexandreAmice

@drake-jenkins-bot retest this please

hongkai-dai avatar May 13 '22 17:05 hongkai-dai

When the author is using multiple commits locally (rather than always amending to a single commit), then our advice should be to merge upstream/master into the PR branch, not to rebase.

That ship has sailed here, though. Whatever we can do to recover is fine.

jwnimmer-tri avatar May 13 '22 18:05 jwnimmer-tri

Thanks Jeremy, so for this PR with this merge commit, reviewable shows that 341 files need to be reviewed, should the reviewer find out the actual change out of these 341 files?

Yes, this PR is broken and needs a force-push after being clean up locally.

I suggested doing rebase which I believe also works fine with multiple local commits, ...

In general, it does not. At this point (now that everything is broken), locally rebasing and then squashing is probably easiest. But in the future, when the author is using multiple commits, we should ask them to merge. If they do that (without rebasing), reviewable is 100% fine.

jwnimmer-tri avatar May 13 '22 18:05 jwnimmer-tri

I really have no idea. If I need to give advice, I'd really just fix it locally and then force-push into the branch here myself. That seems easier than trying to describe it.

Maybe even just hard resetting to the prior revision would be easiest, via git fetch upstream refs/reviewable/pr16066/r7 && git checkout FETCH_HEAD and then force push.

jwnimmer-tri avatar May 13 '22 18:05 jwnimmer-tri

Would you still like to have Russ on feature review?

jwnimmer-tri avatar May 16 '22 18:05 jwnimmer-tri

I think that was just Xuchen making connections. I don't feel needed here. -@russtedrake .

RussTedrake avatar May 16 '22 18:05 RussTedrake

FYI it was from @AlexandreAmice actually: image

jwnimmer-tri avatar May 16 '22 18:05 jwnimmer-tri

@alexandreamice -- ping. it would be great to not let this get too stale.

RussTedrake avatar Aug 15 '22 09:08 RussTedrake