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

delete other filesの処理を修正 #47

Merged
merged 4 commits into from
Dec 7, 2023
Merged

delete other filesの処理を修正 #47

merged 4 commits into from
Dec 7, 2023

Conversation

theoremoon
Copy link
Member

#45 の修正が不完全だったので追い修正です

  • grep -xL "EditURL" $(git ls-files) 形式だと、 git ls-files の結果がなかったときgrepがstdinから読もうとしてしまいそうなのでxargs -rを利用するように(-r--no-run-if-empty の短縮形)
  • grepはマッチしなかったときにステータスコードを0以外にするので -e-o pipefail をなしに
  • typo修正

@theoremoon theoremoon requested a review from halkt December 6, 2023 11:45
for file in ${delete_files[@]}; do
rm "$file"
done

restore_files=($(grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}" $(git ls-files -m --exclude-standard)))
for file in ${resotre_files[@]}; do
restore_files=(git ls-files -m --exclude-standard | xargs -r grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}")
Copy link
Member

Choose a reason for hiding this comment

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

[must]$が抜けてそうでした.

Suggested change
restore_files=(git ls-files -m --exclude-standard | xargs -r grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}")
restore_files=$(git ls-files -m --exclude-standard | xargs -r grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}")

@@ -29,13 +29,14 @@ runs:
shell: bash
- name: delete other files
run: |
delete_files=($(grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}" $(git ls-files -o --exclude-standard)))
set +eo pipefail
delete_files=$(git ls-files -o --exclude-standard | xargs -r grep -xL "EditURL: ${{ steps.set-entry-variables.outputs.EDIT_URL }}")
Copy link
Member

Choose a reason for hiding this comment

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

[should]restore_files の方もですが, ()で括っていないので配列じゃなくなっていそうです.

Copy link
Member Author

Choose a reason for hiding this comment

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

なるほど……

@theoremoon theoremoon requested a review from halkt December 7, 2023 02:30
@theoremoon
Copy link
Member Author

theoremoon commented Dec 7, 2023

レビューありがとうございます。2箇所とも治したほうが良さそうだったので治しました

Copy link
Member

@halkt halkt left a comment

Choose a reason for hiding this comment

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

良さそうです~

@theoremoon
Copy link
Member Author

dmdm~

@theoremoon theoremoon merged commit 2adba7c into main Dec 7, 2023
@theoremoon theoremoon deleted the fix-git-restore branch December 7, 2023 02:54
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