Review seems to assume the base branch is `main`

getappmap / appmap-js

Client libraries for AppMap

48 stars 17 forks source link

Review seems to assume the base branch is `main` #2129

Closed dividedmind closed 2 hours ago

dividedmind commented 2 hours ago

I tried the following in our vscode-appland repository: obraz

A couple of notes:

it seems to assume the base is main,
it runs git with system locale. If we're consuming git output in code, we should probably run it with C locale and/or try to use low-level commands with guaranteed stable output (though this might not apply to error messages — however maybe we should verify first to make sure a command won't fail),
it also appears hung — it probably crashed; I'd expect some kind of error recovery.

kgilpin commented 2 hours ago

You don’t have to guess - there is documentation

https://appmap.io/docs/navie-reference/navie-commands.html#review

kgilpin commented 2 hours ago

I am closing this because the behavior re: main branch is as documented.

You can open separate issues if you believe you have found a bug.

Confirm the bug and provide a reproduction if you want to report something. As an appmap developer, I don’t want to see guesses - eg “it appears hung”.

dividedmind commented 1 hour ago

I just noticed that while doing something else and thought I'd open a bug report because from the point of view of a user it definitely looks broken. Figured it'd be worth it to track this and flesh this out later. It does appear hung to the user, it's a statement of fact, not a guess; the suggest box keeps spinning. Are you saying there's no value in documenting these kinds of UX bugs for tracking unless I can fully flesh them out?

(BTW, you closed this as completed; I think you meant to close as invalid/wontfix.)