Detect deletion of slides #1430
Conversation
|
any suggestions🙂 ? |
| - name: List deleted slides | ||
| id: deleted-slides | ||
| run: | | ||
| deleted_slides=$(git diff --name-only HEAD^ HEAD) |
There was a problem hiding this comment.
I think this will include all modified slides, right?
Also, this only compares HEAD^ to HEAD -- what if the PR has multiple commits in it?
There was a problem hiding this comment.
updated it in my new commit .
| for slide in $deleted_slides; do | ||
| slide_filename=$(basename -- "$slide") | ||
| slide_name="${slide_filename%.*}" | ||
| entry_exists=$(grep -c "\[slide.$slide_name\]" book.toml) |
There was a problem hiding this comment.
I'm not sure what this pattern is intended to match. It looks like a character class?
There was a problem hiding this comment.
The pattern "\[slide.$slide_name\]" is not intended to create a character class. Instead, it is meant to match a string in the format of [slide.slide_name] where slide_name is a placeholder for the actual name of the slide.
There was a problem hiding this comment.
Ah -- and where does that appear in book.toml?
|
Oh, and please update the title of the PR to reflect what it does. |
| base_branch=$(jq -r .pull_request.base.ref "$GITHUB_EVENT_PATH") | ||
| echo "base_branch=$base_branch" >> $GITHUB_ENV | ||
| else | ||
| echo "base_branch=" >> $GITHUB_ENV |
There was a problem hiding this comment.
This will end up running git diff ...HEAD on anything that is not a pull request, which does nothing. If that is the intent, perhaps this action should only run on pull requests?
|
|
||
| - name: Set deleted slides in an environment file | ||
| run: | | ||
| base_branch="${{ env.base_branch }}" |
There was a problem hiding this comment.
What is the reason to spread this processing over multiple workflow steps? Couldn't this all be accomplished in a single step?
|
sry @djmitche for late reply ... will soon come up with changes |
|
Hi @Gourav2609, thanks for looking at this issue! As a high-level comment, I would like code such as this to be written in Python. See https://github.com/google/comprehensive-rust/blob/main/.github/workflows/check-msgid-changes.yml which calls directly out to https://github.com/google/comprehensive-rust/blob/main/.github/workflows/check-msgid-changes.py. We cannot maintain large amounts of Bash code, so that is why I think we should write this in a high-level language instead. |
|
@Gourav2609 are you able to continue working on this? |
|
@djmitche sry.. Got a little busy with college hackathon.. Will soon start working on it |
closes issue #1417