Skip to content
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

added DecimalToHex #73

Merged
merged 5 commits into from Mar 21, 2019
Merged

added DecimalToHex #73

merged 5 commits into from Mar 21, 2019

Conversation

@AlexDvorak
Copy link
Contributor

AlexDvorak commented Oct 20, 2018

contributes to #14

@ParthS007
Copy link
Member

ParthS007 commented Oct 22, 2018

This pull request introduces 1 alert when merging 859a83b into 2d50357 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Comment posted by LGTM.com

@AlexDvorak AlexDvorak force-pushed the AlexDvorak:master branch from 859a83b to 31d5467 Oct 22, 2018
Copy link
Member

ms10398 left a comment

Please fix the alert posted above

@AlexDvorak
Copy link
Contributor Author

AlexDvorak commented Oct 23, 2018

@ms10398 github says it’s up to date when i push but it isn’t and luhn.js should be in another branch

@AlexDvorak
Copy link
Contributor Author

AlexDvorak commented Oct 23, 2018

@ms10398 problem resolved LGTM

@christianbender
Copy link
Contributor

christianbender commented Feb 17, 2019

@AlexDvorak BUG:

console.log(decimalToHex(125)); // should be 7D but it was D

FIX

Change your function decimalToHex(...) this way:

function decimalToHex(num){
    let hex_out = [];
    while(num > 15) {
        hex_out.push(intToHex(num%16));
        num = Math.floor(num / 16);
    }
    
    return intToHex(num) + hex_out.join("");  // BUG FIX
}
@christianbender
Copy link
Contributor

christianbender commented Feb 17, 2019

@AlexDvorak Please seperate your pull request in multilpe PRs. One for each filing.

Checksums/luhn.js Outdated Show resolved Hide resolved
@ParthS007
Copy link
Member

ParthS007 commented Feb 24, 2019

This pull request introduces 1 alert when merging 97ac1c3 into 9cabd46 - view on LGTM.com

new alerts:

  • 1 for Syntax error

Comment posted by LGTM.com

@ms10398 ms10398 merged commit ad001c4 into TheAlgorithms:master Mar 21, 2019
1 check passed
1 check passed
LGTM analysis: JavaScript No new or fixed alerts
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.