[Graphite] Add support for this cursed tool to track#221
[Graphite] Add support for this cursed tool to track#221gbalke wants to merge 1 commit intoSkydio:mainfrom
Conversation
I don't really want to talk about how graphite manages metadata... I did what had to be done to make this work. Signed-off-by: Greg Balke <greg@physicalintelligence.company> Topic: make_stacks_work_in_graphite Reviewers: jerry
gbalke
force-pushed
the
main
branch
2 times, most recently
from
a2ac8e7 to
7d7d691
Compare
|
@aaron-skydio if you have time to give a quick pass 🙏 |
|
This makes sense to me, but I'm probably not going to make a new release until @jerry-skydio 's back either way - does it help you to have this in main before then or can it just wait for Jerry? |
|
Yeah I can wait for jerry. Thanks for looking. I just wanted it in main so I could build a devblob equivalent locally that's targetting something that's on mainline. |
| upload_parser.add_argument("--uploader") | ||
| upload_parser.add_argument( | ||
| "--branch-format", choices=["user+branch", "user", "branch", "none"], default="user+branch" | ||
| "--branch-format", |
There was a problem hiding this comment.
divisors? that doesn't really make sense maybe you mean dividers?
although i think the choice of divider character is actually orthoganal to branch format, which defines what things are incorporated into the name. ie you could use underscores with user+branch, user, branch, etc.
i think what this actually needs is a new argument that is "--branch-divider" defaulting to "/". you can of course set it to anything (although maybe it makes sense to use a different default if graphite is enabled)
| Graphite closes/deletes branches instead of merging, so we need to | ||
| rebase to clean up local history. | ||
| """ | ||
| logging.info("Fetching and rebasing to drop already-merged commits") |
There was a problem hiding this comment.
i'm a bit confused by this -- why doesn't "git pull --rebase" work? "revup upload" has always operated on the invariant that an upload does not change your local working state, so i don't think uploading should do a rebase for you -- that should be a separate thing done by the user since it could result in conflicts, trash your build state, take a long time, etc
|
this needs significantly more detail in the PR message. I need to know how graphite works (at least somewhat basically) in order to do this review. if graphite does not provide open source docsyou can link to, i need a summary in the PR body. its fine for you to make any changes you need in your fork, but remember that by submitting to an open source project you're asking that project to adopt and maintain your changes essentially forever, so the standards for documentation are much higher than for an internal change |
3 participants
I don't really want to talk about how graphite manages metadata... I did what had to be done to make this work.
Signed-off-by: Greg Balke greg@physicalintelligence.company
Topic: make_stacks_work_in_graphite
Reviewers: jerry