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.

Sign up for GitHub

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

Jump to bottom

add named export to palindrome products #621

Merged
merged 1 commit into from
Mar 4, 2019
Merged

add named export to palindrome products #621

merged 1 commit into from
Mar 4, 2019

Conversation

smb26
Copy link
Contributor

@smb26 smb26 commented Feb 27, 2019

see issue #600

@SleeplessByte
Copy link
Member

I prefer generatePalindromes or simply palindromes (or palindromeProducts) over generate, to be consistent with the other exercises.

tejasbubane
tejasbubane requested changes Feb 27, 2019
View reviewed changes
Copy link
Member

@tejasbubane tejasbubane left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I agree with comment by @SleeplessByte Use any of the given suggestions for function name.

@smb26
Copy link
Contributor Author

smb26 commented Feb 27, 2019

strange, I could swear I added the pull request this morning with the name already changed

@smb26
Copy link
Contributor Author

smb26 commented Feb 27, 2019 •
edited
Loading

Actually, it is the pull request with the requested name (generatePalindrome)

@smb26
Copy link
Contributor Author

smb26 commented Feb 27, 2019

Or is it the request change related to the s (now that I noticed that I used generatePalindrome instead of generatePalindromes)

SleeplessByte
SleeplessByte approved these changes Feb 27, 2019
View reviewed changes
Copy link
Member

@SleeplessByte SleeplessByte left a comment

Choose a reason for hiding this comment

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

@smb26 nope, I don't mind the singular. It wasn't there when I reviewed the first time :)

@smb26
Copy link
Contributor Author

smb26 commented Feb 27, 2019

That's awesome, thank you guys for the opportunity :)

@SleeplessByte SleeplessByte merged commit 540048c into exercism:master Mar 4, 2019
cmccandless pushed a commit to cmccandless/javascript that referenced this pull request Mar 7, 2019
@smb26 @cmccandless
add named export to palindrome products (exercism#621)
eee3056
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SleeplessByte SleeplessByte SleeplessByte approved these changes

@tejasbubane tejasbubane Awaiting requested review from tejasbubane

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@smb26 @SleeplessByte @tejasbubane