-
Notifications
You must be signed in to change notification settings - Fork 183
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
I18n check: fix empty comment #586
Open
spier
wants to merge
40
commits into
main
Choose a base branch
from
i18n-check-fix-empty-comment
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
1f11b0a
Add check for existence of issue.md. Split consistency check and issu…
spier 3c3bc3d
Fix YAML syntax
spier d108c7b
Simulate creation of issue.md (only for Japanese)
spier da92975
Fix bash syntax
spier a2ba3f1
Fix file creation
spier 2b432ec
Remove simulation code
spier 743c384
Combine header for issue, with details about the i18n inconsistencies…
spier d3c7b15
Move issue_title variable to the correct step
spier 7e8f7ca
Remove dummy step
spier be68946
No need to check for issue.md existence within the shell script, as w…
spier cc36c63
Change the search query that identifies the issue to write the data t…
spier 4685428
Add some debug output
spier 61c178f
More debug output. test on the other languages too
spier 7f3ddaf
Add check for existence of issue.md. Split consistency check and issu…
spier 0ea4033
Fix YAML syntax
spier 72cf2df
Simulate creation of issue.md (only for Japanese)
spier a52f65d
Fix bash syntax
spier 70717f3
Fix file creation
spier ce7c677
Remove simulation code
spier 3f09f1e
Combine header for issue, with details about the i18n inconsistencies…
spier 52167a1
Move issue_title variable to the correct step
spier 8e0cd53
Remove dummy step
spier 22bff62
No need to check for issue.md existence within the shell script, as w…
spier 9524620
Change the search query that identifies the issue to write the data t…
spier 746f346
Add some debug output
spier 7a08700
More debug output. test on the other languages too
spier 1ed04bf
Merge branch 'i18n-check-fix-empty-comment' of github.com:InnerSource…
spier 02365ec
Make heredoc work
spier cd539b9
Add more log messages
spier 53eae7a
Change quotes
spier f066ed8
Change more quores
spier 41ed0bb
Repeat flags
spier 247da31
Change sed command to create translation filenames. Add different log…
spier 2d0d04d
Fix error log
spier 23a7611
Ignore all templates for now
spier 54b6c40
Limit search to a single result
spier f2cb5ae
Merge branch 'main' into i18n-check-fix-empty-comment
spier a4d45f5
Determine i18n_filename based on input file
spier 4d24fff
Fix sed syntax
spier 586b683
Also write output for files that are entirely untranslated. Write deb…
spier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,36 +20,41 @@ jobs: | |
- uses: actions/checkout@v3 | ||
with: | ||
fetch-depth: '0' | ||
- name: Check consistency and create issue | ||
- name: Check consistency | ||
id: check-consistency | ||
run: | | ||
# Declare the flags | ||
declare -A flags=( ["ja"]=":jp: Japanese" ["zh"]=":cn: Chinese" ["pt-br"]=":brazil: Brazilian Portuguese" ["gl"]="Galician") | ||
|
||
issue_title="${flags['${{matrix.language}}']}: Content Consistency Issue" | ||
|
||
# Heredoc for issue header | ||
cat <<- EOM > issue.md | ||
# i18n Contents Consistency Issue | ||
|
||
The following files may have consistency issues with the English version. Please check and update the files. | ||
|
||
This issue is created when any of the English patterns have changed (in folder `patterns/`). It compares the git update history to let you know what updates are overdue. The issue should be closed when the update is complete. | ||
EOM | ||
declare -A flags=( ["ja"]=":jp: Japanese" ["zh"]=":cn: Chinese" ["pt-br"]=":brazil: Brazilian Portuguese" ["gl"]="Galician" ) | ||
|
||
# Loop through all files in the English directory | ||
for file in $(find patterns/{2-structured,3-validated} -name '*.md'); do | ||
[[ $file =~ "3-validated" ]] && continue # if the file is under 3-validated, skip (one liner) - 2023/08/26 | ||
i18n_filename=$(echo "$file" | sed "s/patterns\/\(2-structured\|3-validated\)/translation\/${{matrix.language}}\/patterns/g") | ||
|
||
echo "Original file: $file" | ||
#[[ $file =~ "3-validated" ]] && continue # if the file is under 3-validated, skip (one liner) - 2023/08/26 | ||
#[[ $file =~ "/templates" ]] && continue # if the file is under /templates, skip. consistency check does not work for that yet. | ||
|
||
if [[ $file =~ "/templates" ]]; then | ||
i18n_filename=$(echo "$file" | sed -E "s_patterns/(2-structured|3-validated)/templates_translation/${{matrix.language}}/templates_g") | ||
else | ||
i18n_filename=$(echo "$file" | sed -E "s_patterns/(2-structured|3-validated)_translation/${{matrix.language}}/patterns_g") | ||
fi | ||
echo "Translation file: $i18n_filename" | ||
|
||
if [[ ! -e "$i18n_filename" ]]; then | ||
echo "NOT TRANSLATED" | ||
cat <<- EOM >> issue.md | ||
<details><summary><b>New File</b> ($file)</summary> | ||
|
||
For more information, please compare [the original file(en)](https://github.com/$GITHUB_REPOSITORY/blob/master/$file). | ||
</details> | ||
EOM | ||
continue | ||
fi | ||
|
||
original_updated_at=$(date -d "$(git log -1 --format=%cd --date=iso $file)" +%s) | ||
i18n_content_updated_at=$(date -d "$(git log -1 --format=%cd --date=iso $i18n_filename)" +%s) | ||
|
||
if [[ $((original_updated_at - i18n_content_updated_at)) -ge 1 ]]; then | ||
echo "WARNING" | ||
content_header=$(grep -E '^title+' "$file" | sort -r | head -n1) | ||
content_title=$(echo "$content_header" | sed 's/title: //g') | ||
|
||
|
@@ -70,20 +75,53 @@ jobs: | |
\`\`\` | ||
</details> | ||
EOM | ||
else | ||
echo "CHECK" | ||
fi | ||
|
||
done | ||
|
||
- name: Check issue.md existence | ||
id: check_files | ||
uses: andstor/file-existence-action@v2 | ||
with: | ||
files: "issue.md" | ||
- name: Create issue | ||
if: steps.check_files.outputs.files_exists == 'true' | ||
run: | | ||
# Declare the flags | ||
declare -A flags=( ["ja"]=":jp: Japanese" ["zh"]=":cn: Chinese" ["pt-br"]=":brazil: Brazilian Portuguese" ["gl"]="Galician" ) | ||
|
||
# Heredoc for issue header | ||
cat <<- EOM > issue-full.md | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than introducing a new file, we could also try to prepend the header to the content of issue.md. Logic in pseudo code:
Just not sure how to do this in a shell script :) |
||
# i18n Contents Consistency Issue | ||
|
||
The following files may have consistency issues with the English version. Please check and update the files. | ||
|
||
This issue is created when any of the English patterns have changed (in folder `patterns/`). It compares the git update history to let you know what updates are overdue. The issue should be closed when the update is complete. | ||
EOM | ||
|
||
# combine header and rest of file created in previous steps | ||
cat issue.md >> issue-full.md | ||
|
||
# title of the GitHub issue to look for | ||
issue_title="${flags['${{matrix.language}}']}: Content Consistency Issue" | ||
|
||
# Get the existing issue ID | ||
existing_issue_id=$(gh issue list -S "is:issue is:open $issue_title" | cut -f1) | ||
search_query="is:issue is:open in:title \"$issue_title\"" | ||
echo "search_query: $search_query" | ||
existing_issue_id=$(gh issue list -L 1 -S "$search_query" | cut -f1) | ||
echo "existing_issue_id: $existing_issue_id" | ||
|
||
cat issue-full.md | ||
|
||
# Create a new issue or comment on the existing one | ||
if [ -f issue.md ]; then | ||
if expr "$existing_issue_id" : '^[0-9]*$' >/dev/null; then | ||
gh issue comment "$existing_issue_id" -F issue.md -R $GITHUB_REPOSITORY | ||
else | ||
gh issue create -t "$issue_title" -F issue.md -R $GITHUB_REPOSITORY -l "Type - Translation" | ||
fi | ||
fi | ||
#if expr "$existing_issue_id" : '^[0-9]*$' >/dev/null; then | ||
# gh issue comment "$existing_issue_id" -F issue-full.md -R $GITHUB_REPOSITORY | ||
#else | ||
# gh issue create -t "$issue_title" -F issue-full.md -R $GITHUB_REPOSITORY -l "Type - Translation" | ||
#fi | ||
|
||
env: | ||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if a GHA is needed to check for file existence.
Maybe this can be done directly with a GHA expression? Could not find the syntax for that though.
Also assumed that the file-existence-action probably was created for a reason (i.e. that it was not possible differently).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This implementation was set up in the past when we needed to do conditional calls on the GitHub Actions side, but as far as the current implementation is concerned, it only does the work when True, so I don't think we need this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cross post #636 here
I edited this. Please merge this 🙏
I think my change is what you meant here. I did not push directly because I am not a bit confident about it, but I used PR!