Skip to content

Refactor comparison function.#47

Merged
sethvincent merged 2 commits into
workshopper:masterfrom
SomeoneWeird:refactor-compare
Jan 17, 2015
Merged

Refactor comparison function.#47
sethvincent merged 2 commits into
workshopper:masterfrom
SomeoneWeird:refactor-compare

Conversation

@SomeoneWeird
Copy link
Copy Markdown
Member

@SomeoneWeird SomeoneWeird commented Jan 12, 2015

Do not merge this PR yet, it is not finished

Just going to try and gauge ideas etc again.

This moves the solution of each (only introduction exercise atm) into it's own file, this also moves the comparison of solution & attempt into it's own file.

There is also a diff that is provided (like learnyounode) when the solution is incorrect (the format of the output could definitely use with some work).

Is this something that we want to do?

screenshot 2015-01-12 22 17 19

@tomgco
Copy link
Copy Markdown
Contributor

tomgco commented Jan 12, 2015

Awesome, was just about to look into this myself!

The diff for me is really important, its pretty much impossible to figure out mistakes without it.

@sethvincent
Copy link
Copy Markdown
Collaborator

sethvincent commented Jan 12, 2015

Yeah, this looks great!

The diffs are going to be really useful.

@SomeoneWeird SomeoneWeird force-pushed the refactor-compare branch 2 times, most recently from bef1281 to ab7b36e Compare January 14, 2015 09:59
@SomeoneWeird
Copy link
Copy Markdown
Member Author

SomeoneWeird commented Jan 14, 2015

OK, this is ready to be merged. I updated it so the comparison is agains the solution in ./solutions instead of copying it again, added a couple errors checks, and of course updated the rest of the problems.

Lemme know if something looks bad :)

@sethvincent
Copy link
Copy Markdown
Collaborator

sethvincent commented Jan 14, 2015

This is awesome! Trying it out right now.

@sethvincent
Copy link
Copy Markdown
Collaborator

sethvincent commented Jan 14, 2015

What do you think about changing the order slightly?

I found myself automatically looking at the line that starts with a # and missed the solution, attempt, diff stuff at first.

So instead of this:

Solution:
------------------------
hello

Your attempt:
------------------------
hi

Difference:
------------------------
hiello

-------------------
# O-oh, something isn't working.

But don't panic!
Check next things:
  1) did you type the name of the file correctly? You can check by running ls introduction.js, if you see ls: cannot access introduction.js: No such file or directory then you should create new file / rename existing or change directories to the one with file
  2) make sure you didn't omit parens, since otherwise compiler would not be able to parse it
  3) make sure you didn't do any typos in the string itself

-------------------
Need help? Ask a question at: github.com/nodeschool/discussions/issues

It would be this:

# O-oh, something isn't working.

But don't panic!

## Check your solution:

Solution:
------------------------
hello

Your attempt:
------------------------
hi

Difference:
------------------------
hiello

-------------------

## Additional troubleshooting:
  1) did you type the name of the file correctly? You can check by running ls introduction.js, if you see ls: cannot access introduction.js: No such file or directory then you should create new file / rename existing or change directories to the one with file
  2) make sure you didn't omit parens, since otherwise compiler would not be able to parse it
  3) make sure you didn't do any typos in the string itself

-------------------
Need help? Ask a question at: github.com/nodeschool/discussions/issues

@SomeoneWeird
Copy link
Copy Markdown
Member Author

SomeoneWeird commented Jan 15, 2015

Yeah, the problem there is all that stuff gets automatically spat out when you pass false to the callback, and i'm just console.log'ing the stuff at the start. Will take a look in a bit.

@SomeoneWeird
Copy link
Copy Markdown
Member Author

SomeoneWeird commented Jan 16, 2015

@sethvincent

OK! I've refactored a bunch more stuff, every problem index.js is now generic and the same, and there is only 1 troubleshooting.md file now which stuff gets injected into. I also removed the blank readme.md's from the problem folders.

It's a little bit hacky, but it works for what we need, and makes future improvement quite easy.

(i can squash these commits if you want, before you merge)

@sethvincent
Copy link
Copy Markdown
Collaborator

sethvincent commented Jan 17, 2015

Niiiiice. This is great. I'll go ahead and merge it. Adding you as a collaborator, too, if that's alright with you.

sethvincent added a commit that referenced this pull request Jan 17, 2015
@sethvincent
Merge pull request #47 from SomeoneWeird/refactor-compare
dc2c6e8 
Refactor comparison function.
@sethvincent sethvincent merged commit dc2c6e8 into workshopper:master Jan 17, 2015
@sethvincent
Copy link
Copy Markdown
Collaborator

sethvincent commented Jan 17, 2015

on npm as v1.10.0

@SomeoneWeird
Copy link
Copy Markdown
Member Author

SomeoneWeird commented Jan 17, 2015

Awesome!

@SomeoneWeird SomeoneWeird deleted the refactor-compare branch January 17, 2015 23:56
SomeoneWeird added a commit that referenced this pull request Jan 19, 2015
@SomeoneWeird
fixes windows issue introduces in #47. closes #50
85211df
SomeoneWeird added a commit that referenced this pull request Jan 19, 2015
@SomeoneWeird
fixes windows issue introduced in #47. closes #50
fcd3c31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@SomeoneWeird @tomgco @sethvincent