Closed jsteinshouer closed 6 years ago
Excellent work @jsteinshouer ! I added some comments. Nothing big, just some suggestions and improvements. Let me know what you think.
@bdw429s Thanks for the feedback. I will get those changes made.
Thanks @jsteinshouer -- please note I merged a pull last night that provides a task runner for normalizing the JSON config file (and requires CommandBox 3.9.0-snapshot
). It modifies a line in the excercise scaffold so you'll want to rebase your work and make sure that merges ok. It shouldn't cause any issues since the line I touched isn't close to the additions you made.
I may have botched the rebase because I ended up with a conflict when trying to push to github. Let me know if it would be better to delete my branch and start a new one.
Not sure Hmm, not sure what happened. I assume you wanted to:
Usually "extra" commits that make it into pulls don't show in the diff since the commits are already in the repo, so I'm not sure right off why the diff shows all the files from those commits. I'll try a merge and it if seems completely screwy, I'll back it out and we can try from scratch.
@jsteinshouer Well, it all seems kosher. When I merged the upstream into my fork, it came across as a single commit that only has your changes so I think we're good.
@jsteinshouer Awesome, I just used your task runner to generate all the readmes in all exercises just to compare the diff to what was there before. In all but one case, they are identical except a bit of whitespace or line endings.
The only exception appears to be a character encoding issue on the pangram
exercise. The original readme had:
Determine if a sentence is a pangram. A pangram (Greek: παν γράμμα, pan gramma,
But the new readme has:
Determine if a sentence is a pangram. A pangram (Greek: παν γ�?άμμα, pan gramma,
Do you want to take a look at that and see what is going on?
I am relieved that it merged. I will look into the issue with the panagram
exercise. Thanks!
@bdw429s Here is my attempt at the README task runner. Hope this looks ok.