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

Checklist: convert export default to named export #600

Closed
7 tasks done
SleeplessByte opened this issue Jan 24, 2019 · 10 comments
Closed
7 tasks done

Checklist: convert export default to named export #600

SleeplessByte opened this issue Jan 24, 2019 · 10 comments
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. good first issue help wanted

Comments

@SleeplessByte
Copy link
Member

SleeplessByte commented Jan 24, 2019

Follow up on #436 and the implementation #599 .

  • variable-length-quantity
  • strain
  • palindrome-products
  • queen-attack
  • sum-of-multiples
  • sieve
  • hexadecimal
@SleeplessByte SleeplessByte added the chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. label Jan 24, 2019
SleeplessByte pushed a commit that referenced this issue Jan 24, 2019
Instead of default exports

Issue: #436
Remainder: #600
@smb26
Copy link
Contributor

smb26 commented Feb 12, 2019

hello, can I have this task? I'll start with the first 3 ones: variable-length-quality, strain and palindrome-products. If things goes well I can do the remaining ones.

@SleeplessByte
Copy link
Member Author

@smb26 sure! Just edit your message and say which one you're fixing :)

@smb26
Copy link
Contributor

smb26 commented Feb 12, 2019

Thank you!

@smb26
Copy link
Contributor

smb26 commented Feb 14, 2019

Hey @SleeplessByte, can I do a single pull request with the changes on all the six remaining exercises or should I keep doing separate pull requests?

@SleeplessByte
Copy link
Member Author

I prefer separate @smb26 , but you can just keep them all open at the same time.

@smb26
Copy link
Contributor

smb26 commented Feb 27, 2019

Hello, I have a doubt for the palindrome exercise - on the example, what is being exported is a generate function that will return two objects: largest and smallest, each with value and factors.

To change it for a named export, should I split it, ending up with two methods that each will return an object?

@sin
Copy link

sin commented Feb 27, 2019

To change it for a named export, should I split it, ending up with two methods that each will return an object?

I'd say no. It's outside of the scope of converting default export to named export. It would change how the solution works internally, not only how the code is exported. You can see in the example that it uses the same loop to generate both results. These calculations are interlinked and it makes sense to do them in one function. If you think the exercise would be better if split into two separate functions, you can present your arguments – you may be right, maybe it should be split, but I think it's a separate issue.

@smb26
Copy link
Contributor

smb26 commented Feb 27, 2019

Honestly, I don't feel like it has something to be split, but since the run-length had a separation between functions, I decided to ask about the best strategy. Thank you @sin

@SleeplessByte
Copy link
Member Author

@smb26 Right now it's expecting a single function as default export. When you do this one, I would make it maybe expect palindromes(opts) => ....

- const Palindromes = (params) => {
+ export const palindromes = (params) => {
  ...
- export default Palindromes;

@SleeplessByte
Copy link
Member Author

A humongous thanks to @smb26 ! Thank you for making this happen 💟

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. good first issue help wanted
Projects
None yet
Development

No branches or pull requests

3 participants