Skip to content

Convert scripts to ES6#687

Merged
SleeplessByte merged 2 commits intoexercism:masterfrom
tejasbubane:scripts-es6
Jun 13, 2019
Merged

Convert scripts to ES6#687
SleeplessByte merged 2 commits intoexercism:masterfrom
tejasbubane:scripts-es6

Conversation

@tejasbubane
Copy link
Copy Markdown
Member

Use @babel/node to run them.

Also fix comments in scripts.

Copy link
Copy Markdown
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.

- function prepareAndRun(command, infoStr, failureStr) {
+ const prepareAndRun = (command, infoStr, failureStr) => {

Any reason you changed from function declaration to "assignment pattern" + arrow function expression? I know it's mostly a stylistic choice, but I think the explicit function reads better 🥇

Also, if you're not re-assigning variables, can you use const instead of let?

❤️ for this PR!

@tejasbubane
Copy link
Copy Markdown
Member Author

Any reason you changed from function declaration to "assignment pattern" + arrow function expression? I know it's mostly a stylistic choice, but I think the explicit function reads better

Nothing specific. My personal preference of the shiny arrow function over normal function. I will change it back to function.

if you're not re-assigning variables, can you use const instead of let?

Makes sense. 👍

@SleeplessByte
Copy link
Copy Markdown
Member

I don't mind the const x = () => {}, but I think it helps our junior contributors if it's more straightforward 🗡 I generally use function at the top level, and expression everywhere else.

@tejasbubane
Copy link
Copy Markdown
Member Author

@SleeplessByte Fixed.

@tejasbubane tejasbubane added the chore 🔧 Meta related task such as build, test, linting, maintainers.json etc. label Jun 13, 2019
@SleeplessByte SleeplessByte merged commit bd6e04a into exercism:master Jun 13, 2019
@SleeplessByte
Copy link
Copy Markdown
Member

I think this means we can now do #628 ?

@tejasbubane tejasbubane deleted the scripts-es6 branch June 13, 2019 16:52
@tejasbubane
Copy link
Copy Markdown
Member Author

@SleeplessByte Yup! #688

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants