Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

theme comparison tool: use "git worktree" to build themes #387

Merged
merged 5 commits into from
May 12, 2021

Conversation

mgeier
Copy link
Member

@mgeier mgeier commented Jan 22, 2020

This is not meant to be merged!

This is just a proof of concept, to be able to actually try if the "git worktree" approach could work.

Of course a lot of error checking, comments and other details are missing.

The goal was to implement the essential functionality with as little code as possible.

The version in #379 has many more features, but I just wanted to try the basic stuff.

@s-weigand
Copy link
Contributor

Just tried it and it works nicely 😄

But I found 2 Problems:

  1. If the user removed devtools/_worktree manually, git still knows it and won't create a new worktree.
    This can be solved with (the replace is to support windows):
if not WORKTREE_DIR.exists():
    if str(WORKTREE_DIR).replace("\\", "/") in repo.git.worktree("list"):
        repo.git.worktree("remove", WORKTREE_DIR)
    repo.git.worktree("add", WORKTREE_DIR, "--detach")
  1. New unstaged files are ignored .

@mgeier
Copy link
Member Author

mgeier commented Jan 23, 2020

Thanks for trying this out!

ad 1: I've also seen this problem in the meantime. Your solution should work fine, but probably we could use as_posix() to avoid the manual replace?
We could also try git worktree prune instead of explicitly removing it?

ad 2: I know. As mentioned in #379 (comment), --include-untracked doesn't seem to be available here.
I would just keep this as a known limitation (unless you know a simple solution), I don't think it is a big deal.

Anyway, I just wanted to see here if what I suggested in #379 would actually work and how much code it would need.

Obviously this is much shorter than #379, but of course there are several features missing here.

Do you want to try to make your solution in #379 shorter (or at least simpler) or probably use a few things from here over there?

@s-weigand
Copy link
Contributor

Using as_posix() sounds good, as you saw in my code I use os.path all the time, guess I should use pathlib more often.

I will refactor #379 and use some of your code.

@mgeier
Copy link
Member Author

mgeier commented Apr 29, 2021

I've squashed the commits and made this ready for review.

@s-weigand I've tried to keep your authorship for the documentation.

I think it works really well, except the custom-formats page doesn't seem to be cached successfully (but I guess that's not the fault of this PR).

@mgeier mgeier marked this pull request as ready for review April 29, 2021 18:16
@mgeier mgeier changed the title Proof of Concept: use "git worktree" to build themes theme comparison tool: use "git worktree" to build themes Apr 29, 2021
@mgeier mgeier merged commit be6a081 into spatialaudio:master May 12, 2021
@mgeier mgeier deleted the build-themes-worktree branch May 12, 2021 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants