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 Longest Palindromic Substring in JavaScript #2246

Merged

Conversation

izexi
Copy link
Contributor

@izexi izexi commented Oct 17, 2020

Congrats on taking the first step to contributing to the Sample Programs repository maintained by The Renegade Coder!
For simplicity, please make sure that your pull request includes one and only one contribution.

Please fill one of the sections below as applicable.
Please also add any other relevant information to the Notes section at the bottom.
You may delete or just ignore any other sections.
For more information please refer to our contributing documentation

I Am Adding a New Code Snippet in an Existing Language

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @izexi! Thanks for contributing to this project! We are a rather small team, so it may take some time to process this request. In the meantime, there are several ways you can make yourself a part of The Renegade Coder community. For instance, you can:

Thanks for your help!

Copy link
Member

@jrg94 jrg94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I don't believe we have testing setup yet. Would you be interested in helping us set that up?

@jrg94 jrg94 self-assigned this Oct 17, 2020
@jrg94 jrg94 added enhancement Any code that improves the repo longest palindromic substring See: https://sampleprograms.io/projects/longest-palindromic-substring/ labels Oct 17, 2020
@jrg94 jrg94 added this to the 1,000 Code Snippets milestone Oct 17, 2020
@jrg94 jrg94 added the needs tests New language or project that isn't tested label Oct 17, 2020
@izexi
Copy link
Contributor Author

izexi commented Oct 17, 2020

This looks great! I don't believe we have testing setup yet. Would you be interested in helping us set that up?

Sure, I could look into opening a PR for this sometime tomorrow, just to be clear are you referring to adding a test_longest_palindrome_substring.py test file to the test/projects directory?

@jrg94
Copy link
Member

jrg94 commented Oct 17, 2020

It's actually a somewhat complex process. There are three files that need to be added/updated:

The first two are relatively straightforward. Then, you'd need to add a test_longest_palindrome_substring.py file which includes the actual tests.

@jrg94 jrg94 merged commit 200d9fd into TheRenegadeCoder:master Oct 17, 2020
@jrg94
Copy link
Member

jrg94 commented Oct 17, 2020

One thing worth mentioning is that we might want to add/change tests depending on what makes sense. Also, we need to decided on a file name. Longest Palindromic Substring is a bit long, but I'm not against using that straight up. That said, we've used LCS in the past for Longest Common Subsequence.

@izexi izexi deleted the javascript/longest_palindrome_substring branch October 17, 2020 21:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any code that improves the repo longest palindromic substring See: https://sampleprograms.io/projects/longest-palindromic-substring/ needs tests New language or project that isn't tested
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add Longest Palindromic Substring in JavaScript
2 participants