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

Add latexdiff to artifacts #8

Merged
merged 4 commits into from
Feb 6, 2025
Merged

Add latexdiff to artifacts #8

merged 4 commits into from
Feb 6, 2025

Conversation

dariusptrs
Copy link
Member

No description provided.

Copy link

github-actions bot commented Feb 5, 2025

🤖 The artifacts from this build are available here.

Copy link

github-actions bot commented Feb 5, 2025

🤖 The artifacts from this build are available here.

@dariusptrs
Copy link
Member Author

This workflow change additionally runs latexdiff on the .tex files from the executing branch or fork and compares them to the default branch for easier pull request reviews.

I also noticed that the pull request comment does not work with forks and added a "not-so-fancy" continue-on-error : true workaround.

@hofbi Do you think we should just remove the "Comment on PR with artifact link" step or do you have any other suggestion?

@dariusptrs dariusptrs marked this pull request as ready for review February 5, 2025 20:53
@hofbi
Copy link
Member

hofbi commented Feb 6, 2025

I am fine with removing the comment. It is more a convenience feature, the artifacts are still there.

Regarding the latex diff, could you briefly explain the motivation for this?

For the implementation, I noticed that you are now checking out the base branch. This seems a bit inefficient. I think GH actions provide a very efficient way to give you the actual diff of a PR. Maybe we can make use of this to prevent 2 checkouts.

@dariusptrs
Copy link
Member Author

Alright, I will remove it.

The latex diff should make it more convenient to review changes in a commit or pull request, by running latexdiff on the changed tex file and the original.

I tested this for a current pr in Schaltungstechnik-1-2 and the output is this: Schaltungstechnik_diff.pdf
Red marks deleted text and blue marks newly added text.

I will try to find a more efficient way. But for running latexdiff you usually need the whole .tex files (original and new).

@dariusptrs
Copy link
Member Author

The new approach now only fetches from remote and the files are copied over individually in the for pdf in build/*.pdf loop. Let me know if you like this better.

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@dariusptrs dariusptrs merged commit f2a98a1 into master Feb 6, 2025
2 checks passed
@dariusptrs dariusptrs deleted the feature/latexdiff branch February 6, 2025 14:23
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