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

Improvements to OverleafGitPaperRemote #25

Merged
merged 13 commits into from
Jun 22, 2023
Merged

Improvements to OverleafGitPaperRemote #25

merged 13 commits into from
Jun 22, 2023

Conversation

wrongu
Copy link
Contributor

@wrongu wrongu commented Jun 20, 2023

Motivation for this is to address #19 for OverleafGitPaperRemote by making use of the revision_id field of the edits.

Along the way I also added some tests, including one that essentially gets at the race-condition of #19.

Including some fancy syntax which may or may not be considered good design:

# this now happens in OverleafGitPaperRemote.perform_edit:
with paper.rewind(commit_id, message="applying some edits") as old_paper:
    old_paper._perform_replace(edit)

...and the git history-handling is done when the with block exits

@wrongu wrongu requested a review from j6k4m8 June 20, 2023 17:02
Copy link
Member

@j6k4m8 j6k4m8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh man, this is beautiful! LOVE the context idea!

@wrongu
Copy link
Contributor Author

wrongu commented Jun 20, 2023

Note: tests pass locally on my machine... The tests are failing in GitHub Actions for strange git reasons, maybe having to do with the fact that there is no user.name configured.

Or maybe there is some difference between repo.index.commit (appears to work) and repo.git.commit (appears to be the source of the failing test).

@wrongu wrongu merged commit 7d54b98 into main Jun 22, 2023
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