Skip to content

Conversation

@SpiderMath
Copy link
Contributor

The related question: https://the-algorithms.com/algorithm/sum-of-digits

There's no JavaScript implementation, so I implemented in JS. Also, I might have messed up writing the tests, since it was not exactly clear to me. So I'd request to please crosscheck the tests(, and if I have any mistake/error in there, I'll fix it)

@lgtm-com
Copy link

lgtm-com bot commented Sep 9, 2021

This pull request introduces 1 alert when merging d402327 into f04dec3 - view on LGTM.com

new alerts:

  • 1 for Useless assignment to local variable
@SpiderMath
Copy link
Contributor Author

Fixed the mistake

I intended to initially write a different implementation but I wrote something else 🤦‍♂️
@raklaptudirm
Copy link
Member

What happened to the package-lock.json?

@SpiderMath
Copy link
Contributor Author

Umm, I tried to install the Node Modules which were there for the project (you hadn't told me about Jest testing stuff on Discord yet) and I think the conflict might be because of that.

Also, I'll fix the 2 spaces problem, I thought setting tab space to 2 would suffice but looks like it didn't.

@raklaptudirm
Copy link
Member

For the styling problem, you need to style the code with standard.js.

@SpiderMath
Copy link
Contributor Author

Okay finally. Sorry I have nearly no experience with linters (I just have a eslintrc which use everywhere) so I kinda messed up...

@raklaptudirm
Copy link
Member

standard.js is easy to use with no configuration needed, so in my opinion it is the best for beginners, who are targeted by this repo.

@SpiderMath
Copy link
Contributor Author

I can't help but agree. But as a guy mostly used to ESLint and one who didn't try anything else, it was confusing.

Standard is really good, I don't doubt that. It was mostly my not-being used to it was what kinda messed them up. However, now that I know how it works, I will probably be able to contribute properly next time.

(Also can we merge the PR now? Looks like everything's fine now~)

@raklaptudirm
Copy link
Member

Could you remove the package-lock.json changes?

@SpiderMath
Copy link
Contributor Author

Umm, actually I don't exactly know how I could undo that 😅
Could you please tell me how to do that? ( Sorry if that sounds really dumb 😰 )

@raklaptudirm
Copy link
Member

Just run npm install and then commit it again.

@SpiderMath
Copy link
Contributor Author

Umm, but the thing which caused the changes in the package-lock was me running npm install
I still ran it again, and it generates the same Lockfile to the one I committed (by mistake).

Do you want me to copy the current package-lock from this Repo and put it into mine? Because that's the only other alternative I can think of at the moment.

@raklaptudirm
Copy link
Member

If npm install keeps the file the same, leave it.

@raklaptudirm raklaptudirm merged commit b3b4ad4 into TheAlgorithms:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in progress Being worked on

2 participants