I18n check: fix empty comment #586
Conversation
…e creation into 2 separate steps.
… found during previous steps.
…e already check that in a previous workflow step
spier
added
⚙️ Type - Meta
| if: steps.check_files.outputs.files_exists == 'true' | ||
| run: | | ||
| # Heredoc for issue header | ||
| cat <<- EOM > issue-full.md |
There was a problem hiding this comment.
Rather than introducing a new file, we could also try to prepend the header to the content of issue.md.
Logic in pseudo code:
issue.md = <header string> + issue.md (generated in previous steps)
Just not sure how to do this in a shell script :)
| @@ -72,18 +61,41 @@ jobs: | |||
| EOM | |||
| fi | |||
| done | |||
|
|
|||
| - name: Check issue.md existence | |||
There was a problem hiding this comment.
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.
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.
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!
|
@yuhattor I left this in draft because I think the code can be improved. Also I found it a bit hard to test it end-to-end. I did some basic simulations but cannot say that I am 100% confident that this works as expected. |
spier
requested review from
lenucksi,
NewMexicoKid,
cewilliams and
robtuley
as code owners
|
@yuhattor would be great if you could help me check this PR. |
…o. Don't wrote anything yet.
…e creation into 2 separate steps.
… found during previous steps.
…e already check that in a previous workflow step
…o. Don't wrote anything yet.
…Commons/InnerSourcePatterns into i18n-check-fix-empty-comment
…ug output to console.
|
@spier |
I noticed a bug in
.github/workflows/i18n-consistency-checker.yaml.When it runs but doesn't find any issues, it will still create a comment to the existing GitHub issue.
Example: #583 (comment)
This happened because the script wrote a header to
issue.md, and later one the existence of theissue.mdfile was used to confirm whether an issue/comment should be created or not. As the header always exists, this was always true.I fixes this by breaking up the logic into two main blocks:
a) check for inconsistencies between en and translation
b) if inconsistencies were found, create issue/comment
I am sure that my solution could be greatly improved with some more shell scripting knowledge.
Personally I am not super happy with my introduction of yet another file (
issue-full.md). This could have likely been done differently as well.Looking forward to any input.