-
Notifications
You must be signed in to change notification settings - Fork 662
fix: replace NaN% with 0.0% when all translations complete #1318
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
base: main
Are you sure you want to change the base?
Conversation
The-Best-Codes
left a comment
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.
Just a couple of things I see that you might want to change.
Also, not sure if you ran pnpm run format before committing but don't forget to do that 🙂
package-lock.json
Outdated
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 file needs to be removed, the repo uses pnpm
package.json
Outdated
| "dependencies": { | ||
| "@changesets/changelog-github": "^0.5.0", | ||
| "@changesets/cli": "^2.27.10", | ||
| "build": "^0.1.4", |
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.
Why is this change needed?
|
Hi!!! Glad to see the review
So These are the following changes I made after this email
1. Removed package.json
2. The "build": "^0.1.4" got added due to dependency issue and this is
currently not present in the code
And right now I made sure to run pnpm run format before committing
Thank Youuuu
…On Thu, 30 Oct 2025 at 08:07, BestCodes ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Just a couple of things I see that you might want to change.
Also, not sure if you ran pnpm run format before committing but don't
forget to do that 🙂
------------------------------
On package-lock.json
<#1318 (comment)>
:
This file needs to be removed, the repo uses pnpm
------------------------------
In package.json
<#1318 (comment)>
:
> @@ -27,6 +27,7 @@
"dependencies": {
***@***.***/changelog-github": "^0.5.0",
***@***.***/cli": "^2.27.10",
+ "build": "^0.1.4",
Why is this change needed?
—
Reply to this email directly, view it on GitHub
<#1318 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDTA2HORQJCJWAQACFV6FI332F2YJAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGOJXGIYTKMBYGM>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Hiiiii Lingo.dev
Any update regarding this????
On Thu, 30 Oct, 2025, 12:49 LOKYAAN ABINETRE KOTI, ***@***.***>
wrote:
… Hi!!! Glad to see the review
So These are the following changes I made after this email
1. Removed package.json
2. The "build": "^0.1.4" got added due to dependency issue and this is
currently not present in the code
And right now I made sure to run pnpm run format before committing
Thank Youuuu
On Thu, 30 Oct 2025 at 08:07, BestCodes ***@***.***> wrote:
> ***@***.**** requested changes on this pull request.
>
> Just a couple of things I see that you might want to change.
> Also, not sure if you ran pnpm run format before committing but don't
> forget to do that 🙂
> ------------------------------
>
> On package-lock.json
> <#1318 (comment)>
> :
>
> This file needs to be removed, the repo uses pnpm
> ------------------------------
>
> In package.json
> <#1318 (comment)>
> :
>
> > @@ -27,6 +27,7 @@
> "dependencies": {
> ***@***.***/changelog-github": "^0.5.0",
> ***@***.***/cli": "^2.27.10",
> + "build": "^0.1.4",
>
> Why is this change needed?
>
> —
> Reply to this email directly, view it on GitHub
> <#1318 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BDTA2HORQJCJWAQACFV6FI332F2YJAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTGOJXGIYTKMBYGM>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
@lokyaan I want to test this locally to see if the build dependency is actually needed on my end. AFAIK it works just fine without it. |
|
Yess it works when i tested it again after removing the dependency
…On Thu, 30 Oct, 2025, 23:58 BestCodes, ***@***.***> wrote:
*The-Best-Codes* left a comment (lingodotdev/lingo.dev#1318)
<#1318 (comment)>
@lokyaan <https://github.com/lokyaan> I want to test this locally to see
if the build dependency is actually needed on my end. AFAIK it works just
fine without it.
—
Reply to this email directly, view it on GitHub
<#1318 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDTA2HJ3DTMEXXPLQ7BKO7332JKGDAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINRZGQ3DQNBRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@lokyaan Okay, maybe remove the dependency, then :) |
|
Hiii @lingodotdev/lingo.dev
***@***.***>
I thought I already did it and realised I was looking into another
package.json
I found out what u were referring to and removed the build dependency and
run it again and pushed the code
Thank You
…On Fri, 31 Oct, 2025, 05:27 BestCodes, ***@***.***> wrote:
*The-Best-Codes* left a comment (lingodotdev/lingo.dev#1318)
<#1318 (comment)>
@lokyaan <https://github.com/lokyaan> Okay, maybe remove the dependency,
then :)
—
Reply to this email directly, view it on GitHub
<#1318 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDTA2HO3JCJ4E2BUGHNEGET32KQVLAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINZQG42DGNJQGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@lokyaan Don't forget to run |
|
After running these two commands, I got a lot of files that needs to be
committed. Should I add, commit and push all those to my branch???
I'm sry that this is taking time but I'm a bit confused at this point if
I'm dng that right
…On Fri, 31 Oct 2025 at 21:20, BestCodes ***@***.***> wrote:
*The-Best-Codes* left a comment (lingodotdev/lingo.dev#1318)
<#1318 (comment)>
@lokyaan <https://github.com/lokyaan> Don't forget to run pnpm install to
update the lockfile and pnpm run format to format the codebase again.
After that I think this PR is ready to merge 😀
—
Reply to this email directly, view it on GitHub
<#1318 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDTA2HIOQNN6TOOTWAS7WQD32OAMHAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINZTG4YTIOJRGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
The-Best-Codes
left a comment
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.
Looks good to me!
|
Awesome, thanks!
…On Sat, 1 Nov 2025 at 00:21, BestCodes ***@***.***> wrote:
***@***.**** approved this pull request.
Looks good to me!
—
Reply to this email directly, view it on GitHub
<#1318 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BDTA2HMVXUGNBWF3LKDAVXD32OVSTAVCNFSM6AAAAACKS47ITSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTIMBVG4YDGOJRGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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.
Pull Request Overview
This PR fixes a division-by-zero bug in the lingo.dev status CLI command that caused "NaN%" to appear in the output when all translations are complete (totalWordsToTranslate = 0). The fix adds a guard condition to return 0.0 instead of the result of dividing zero by zero.
Key Changes:
- Added conditional check in status command to handle edge case when
totalWordsToTranslate === 0 - Updated pnpm-lock.yaml with dependency resolution changes (unrelated to main fix)
Reviewed Changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/cli/src/cli/cmd/status.ts |
Added guard condition to prevent NaN% when calculating per-language translation percentages with zero total words |
pnpm-lock.yaml |
Dependency resolution updates for eslint-related packages (unrelated to the main fix) |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const percent = ((words / totalWordsToTranslate) * 100).toFixed(1); | ||
| const percent = | ||
| totalWordsToTranslate === 0 | ||
| ? 0.0 |
Copilot
AI
Nov 12, 2025
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.
The ternary operator has a type inconsistency. The true branch returns 0.0 (a number), while the false branch returns the result of .toFixed(1) (a string). This should be "0.0" (a string) to match the type returned by .toFixed(1).
const percent =
totalWordsToTranslate === 0
? "0.0"
: ((words / totalWordsToTranslate) * 100).toFixed(1);| ? 0.0 | |
| ? "0.0" |
Summary
Fixes a division-by-zero issue in the
lingo.dev statusCLI output.When
totalWordsToTranslateequals 0,(0 / 0) * 100evaluates toNaN, causing the CLI to print:Fix
Added a guard to return
0.0instead ofNaNwhen total words = 0: