-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat: empty string warning #36
Conversation
This was a mistake, meant to open it against my own dev branch and I don't have the option to close as unplanned lmao |
@Keyrxng Just use this pull request as the final one, I converted it to a draft in the meantime. |
@FernandVEYRIER ready when you are mate |
|
id: find_empty_strings | ||
run: | | ||
# Find empty strings and mark them explicitly | ||
output=$(grep -Rn --include=\*.ts "''\|\"\"" --exclude-dir={node_modules,dist,out,tests} . || true) |
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.
You forgot about double back tick ``
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.
fucksake I wish I had seen this earlier but as I said before, me and bash not so great but I'm no quitter 😂
Took me way longer than I wanted it to, it's not 100% perfect neither but it's ~90-95% accurate (my shortcomings relating to bash scripting is the reason why I haven't got it 100%, scripting syntax and sanitization proving tough trying to capture every variation I could (.tsx
being the main point of failure re: {""}
but in tsx terms that's common syntax so I wasn't sure to even capture that but I did anyway)
-
seeing as it's a
commit comment
it's tied to each commit so if""
is used in commit 1 and not resolved in commit 2 then commit 3 will retain the comment symbol but will not receive a comment as the change happened two commits ago, only once""
are removed will the comment symbol disappear. (the 2nd last commit is proof of this, having an comment symbol but no actual comment) -
it's diffed from incoming commit > most recent, the reason why is because if
""
is used in commit 1 and unresolved in commit 2 then every single commit following will receive a comment (spamming if there are multiple) but it won't be tied to any file because the change is in commit 1 so it's just clutter and noise) -
As for implementing an ignore switch, the wf runs on a push to PR so, it only checks for empty strings that are being pushed into the PR not ones that existed prior (unless edited). Disabling prior to running would be
.env
orsecret
based wouldn't it on a per repo basis? I can't think of how to disable it any other way of the top of my head
…into empty-string-warning
683a2cf
to
cdae93a
Compare
Hard to say what went wrong |
Resolves #31
QA: ubq-testing/bot-ai#31