Added pig-latin exercise and Windows Testing Notes #319

sacherjj · 2017-07-17T22:09:03Z

Implementation of the pig-latin exercise. This is much less of a mess than my last pull request. Not sure if difficultly level (2) is right in the JSON. Someone that has more experience where this would fit in might tweak that value, before going live.

I also added a file in _test with instructions for running check-exercises.sh for Windows Rust users. This includes setting up the Ubuntu on Windows sub system and testing against Windows file system hosted files. With this, I was able to run tests to completion on WIndows.

petertseng · 2017-07-17T22:49:47Z

Thanks. It is not yet known when I will have time to review the code, but a reasonable estimate would be sometime between 27 to 48 hours from now (it doesn't have to be me, so feel free to proceed if someone else reviews it).

I would ask that the _test/WINDOWS_README.md be added in its own separate commit (can be a second commit on this PR, or a second PR, your choice) please. It is a separate change from adding pig-latin, and a separate change deserves a second commit.

Feel free to ask for help if you need advice on how to do that. The ways I can think of off the top of my head:

git reset 0a6e7169076eb015e6baee2998fdac6ec55f0ccb; git add exercises/pig-latin; git commit; git add _test/README_WINDOWS.md; git commit
git rm _test/README_WINDOWS.md; git commit; git revert HEAD; git rebase -i 0a6e7169076eb015e6baee2998fdac6ec55f0ccb then squash the rm commit into the "Added pig latin exercise", and give a message like "add Windows testing instructions" to the revert commit
Feel free to use your preferred method of achieving this if you have one that is not listed here; the exact process is not important just the end result.

petertseng · 2017-07-17T22:54:59Z

Not sure if difficultly level (2) is right

Historically, this track has only used difficulties 1, 4, 7, and 10, corresponding to the four sections (Introduction = 1, Getting Rusty = 4, Rust Gets Strange = 7, Putting it all Together = 10) in https://github.com/exercism/rust/blob/aea470a76a9c6f0005adebb2a00d969512da3b6e/problems.md. That file does not exist anymore as it has been replaced with https://github.com/exercism/rust/blob/master/problem_ordering.md . So we are not restricted to only doing 1 4 7 10. But you would be breaking new ground.

So if you think it is 2, all I will ask is that you think pig-latin is harder than the exercises with difficulty 1 and easier than the exercises with difficulty 4. If you think so, I trust your judgment in calling it a 2.

sacherjj · 2017-07-17T23:01:27Z

I'll have to look through the others in difficulty. I've only done the first 10 or so using exercism. I just figured I could get as much practice by making working exercises and help others at the same time, if I picked the easier one. :)

I think this would fit within the 4 difficulty, with combination of regex. I'll get this into two commits with that change.

And I'm doing my other exercises in their own branch, so I hope to keep cleaner.

sacherjj · 2017-07-17T23:15:51Z

I think that is in a good situation now. This has been a little frustrating, but a learning experience. Not unlike the process of learning Rust. Thanks for the help.

Edit: Take two. Lets see if tests for this time. I missed adding .json to the pig-latin commit before.

petertseng

I missed adding .json to the pig-latin commit before.

Probably my bad, I missed config.json in my instructions, sorry about that.

This has been a little frustrating, but a learning experience.

Hoping what you learned will be useful in future open source contributions should you choose to continue to them (this project or others).

Looking pretty good, a number of changes I have to suggest. Feel free to make the changes by amending the existing commits (don't have to be new commits), I'll be able to diff against what I already reviewed.

petertseng · 2017-07-19T03:15:36Z

_test/WINDOWS_README.md

@@ -0,0 +1,52 @@
+# check_exercises.sh for Windows Rust Developers
+
+It is possible to run `check-exercises.sh` on Windows 10, pointing to the Windows location for you GitHub repository.  This is done with the Ubunuto on Windows subsystem.


although it (Ubuntu) is spelled correctly below, not here

petertseng · 2017-07-19T03:16:04Z

_test/WINDOWS_README.md

@@ -0,0 +1,52 @@
+# check_exercises.sh for Windows Rust Developers
+
+It is possible to run `check-exercises.sh` on Windows 10, pointing to the Windows location for you GitHub repository.  This is done with the Ubunuto on Windows subsystem.


Probably "your GitHub repository" rather than "you"

petertseng · 2017-07-19T03:16:09Z

_test/WINDOWS_README.md

+It is possible to run `check-exercises.sh` on Windows 10, pointing to the Windows location for you GitHub repository.  This is done with the Ubunuto on Windows subsystem.
+
+## Enable Developer Mode
+To run Ubunutu on Windows, you need to be in Developer Mode.


although it (Ubuntu) is spelled correctly below, not here

petertseng · 2017-07-19T03:16:23Z

_test/WINDOWS_README.md

+
+## Install 
+
+Start a PowerShell as Administartor.


Administrator

petertseng · 2017-07-19T03:17:13Z

_test/WINDOWS_README.md

+
+This will redownload and build any crates needed, as they only existed in your Windows Rust.
+
+


I think I would remove these extra newlines (leaving just one trailing newline at the end of the file)

petertseng · 2017-07-19T03:18:38Z

config.json

@@ -660,6 +660,16 @@
      ]
    },
    {
+      "uuid": "c21c379b-fb23-449b-809a-3c6ef1c31221",
+      "slug": "pig-latin",


Since the exercises will be delivered in the order the appear in this exercises array, it seems to make the most sense to move this exercise somewhere with the other difficulty 4 exercises.

petertseng · 2017-07-19T03:21:14Z

exercises/pig-latin/README.md

+
+## Source
+
+Rust System: Tyler Long


this README will probably get regenerated, which means this custom content will get lost (it will simply become the info located at https://github.com/exercism/problem-specifications/blob/master/exercises/pig-latin/metadata.yml ). I don't need you to do anything about this, just letting you know it will happen. (Your name will remain in the Cargo.toml though)

petertseng · 2017-07-19T03:24:27Z

exercises/pig-latin/Cargo.toml

@@ -0,0 +1,8 @@
+[package]
+name = "pig-latin"
+version = "0.1.0"


since this exercise complies with https://github.com/exercism/problem-specifications/blob/master/exercises/pig-latin/canonical-data.json which is version 1.0.0, could you change this version here to 1.0.0?

It is not your fault that you did not know to do this, since this track has failed to document it (#278 (comment))

petertseng · 2017-07-19T03:27:51Z

exercises/pig-latin/Cargo.toml

+authors = ["sacherjj <[email protected]>"]
+
+[dependencies]
+regex = "0.2"


since it is not necessarily the case that any student's solution will use either regex or lazy_static, could you move these dependencies to a file named Cargo-example.toml? See https://github.com/exercism/rust/blob/master/exercises/alphametics/Cargo-example.toml versus https://github.com/exercism/rust/blob/master/exercises/alphametics/Cargo.toml for an example. Otherwise, a student may see these dependencies and not know what to do with them, or think that using them is mandatory and that would unintentionally narrow the kinds of solutions we might see submitted.

petertseng · 2017-07-19T03:30:37Z

exercises/pig-latin/Cargo.lock

+[root]
+name = "pig-latin"
+version = "0.1.0"
+dependencies = [


related to the move to Cargo-example.toml, I would make the Cargo.lock based off of an empty dependencies in Cargo.toml so that the student does not see dependencies that they might not use

sacherjj · 2017-07-19T04:19:51Z

Well I thought I did that as amending, but ran into a conflict. Not sure how I got two commits. I think the changes are all there. Let me know if I need to run some commands to fix this. Worried that if I do anything, I'll make it worse.

ijanos

looks good to me. thank you!

sacherjj · 2017-07-19T16:22:20Z

Thanks for the help Peter. I'll try to make this smoother next time. 👍

petertseng · 2017-07-19T21:23:37Z

Well I thought I did that as amending, but ran into a conflict. Not sure how I got two commits

It looks like all changes you made (changes to both the pig-latin exercise and the Windows instructions) got amended into the Windows instructions commit. At that point, your local history looks like:

M-A-B

M is exercism's master, A is your pig-latin commit, B is your amended windows commit (which includes pig-latin changes)

Your origin/master (your fork) looks like this:

M-A-B'

M and A are the same, but B' is the unamended version of your Windows README.

When you merge with origin/master, you get:

    B'
   /  \
M-A    merge
   \  /
    B

There is a merge conflict here because B and B' are trying to do two different things to _test/README_WINDOWS.md.

When amending a commit, it naturally causes a conflict when merging with the upstream branch (the amended commit and unamended commit are doing two different things to the same files), so you do not merge with the upstream branch, you should simply overwrite it, so that it looks like M-A-B.

I think the changes are all there.

👍 Yup.

It also would make sense for pig-latin changes to get amended into the pig-latin commit, instead of into the Windows README commit, because they are separate changes and the changes should go to the appropriate commits.

A possible way to achieve this would be to do a similar procedure as we did before: git reset 0a6e7169076eb015e6baee2998fdac6ec55f0ccb; git add config.json; git add exercises/pig-latin; git commit; git add _test/README_WINDOWS.md; git commit; git push --force-with-lease.
(0a6e716 is the M in the above diagrams. I deterined this by looking at git log on this branch and it was the latest commit that was not made by you (not in this pull request))

Another way would have to initially make the updates to pig-latin in a separate commit, and the updates to the README_WINDOWS in another separate commit, then git rebase -i 0a6e7169076eb015e6baee2998fdac6ec55f0ccb) and squash the commits so that there is one commit for pig-latin and one commit for README_WINDOWS.

Worried that if I do anything, I'll make it worse.

If you do make it worse, it is always possible to return to the previous state. I noted at the time that the commit made was 91a1009, so it would have been possible to do git reset --hard 91a100936def5f5fa5ce919ea3cdfdc00e63f657 to return to that state.

petertseng · 2017-07-19T21:45:34Z

Thanks for the new exercise and the testing instructions.

I moved the changes into two commits. The disadvantage is that you don't get the practice of doing so. If you are interested in doing that, you may feel free to recreate the situation, I imagine it would be something like:

git checkout -b testconflicts
echo acontents > a
git add a
git commit
echo bcontents > b
git add b
git commit
git push -u origin testconflicts
echo addsomethingtoa >> a
echo completelychangeb > b
git commit -a --amend

At this point, you would get a conflict if you git pull or git merge origin/testconflicts. Thus you do not want to run either of those commands, you want to git push --force-with-lease.

It may also be useful to cause the conflict (run git pull), and fix it (I suggest using git reset with similar procedures as what I described above)

petertseng · 2017-07-21T18:55:10Z

Now I wonder whether there are any topics that can be added. lack of topics causes extra newline in #322. probably something to do with strings. shrug

petertseng · 2017-07-21T19:00:22Z

I forgot to ask for an empty src/lib.rs file (#270)

sacherjj · 2017-07-21T19:03:21Z

.gitignore would need to be modified for src/lib.rs. It currently has all of src ignored.

petertseng · 2017-07-21T19:04:38Z

indeed, I would remove src from gitignore.

Forgot to be added in #319. Should be added because of #270.

sacherjj force-pushed the master branch from f414ae4 to 8ec27da Compare July 17, 2017 23:12

sacherjj force-pushed the master branch from 8ec27da to 5638a5f Compare July 17, 2017 23:43

petertseng reviewed Jul 19, 2017

View reviewed changes

ijanos approved these changes Jul 19, 2017

View reviewed changes

petertseng force-pushed the master branch from 91a1009 to f4c4c92 Compare July 19, 2017 21:09

sacherjj added 2 commits July 19, 2017 14:09

pig-latin: Add exercise to Rust track

d7814f7

_test: Add help for Windows developers to run tests

7496703

petertseng force-pushed the master branch from f4c4c92 to 7496703 Compare July 19, 2017 21:11

petertseng merged commit e290813 into exercism:master Jul 19, 2017

petertseng pushed a commit that referenced this pull request Jul 23, 2017

pig-latin: Add empty src/lib.rs (#324)

c76ec02

Forgot to be added in #319. Should be added because of #270.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added pig-latin exercise and Windows Testing Notes #319

Added pig-latin exercise and Windows Testing Notes #319

sacherjj commented Jul 17, 2017 •

edited

Loading

petertseng commented Jul 17, 2017 •

edited

Loading

petertseng commented Jul 17, 2017

sacherjj commented Jul 17, 2017 •

edited

Loading

sacherjj commented Jul 17, 2017 •

edited

Loading

petertseng left a comment

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

petertseng Jul 19, 2017

sacherjj commented Jul 19, 2017 •

edited

Loading

ijanos left a comment

sacherjj commented Jul 19, 2017

petertseng commented Jul 19, 2017 •

edited

Loading

petertseng commented Jul 19, 2017

petertseng commented Jul 21, 2017

petertseng commented Jul 21, 2017

sacherjj commented Jul 21, 2017

petertseng commented Jul 21, 2017

		@@ -0,0 +1,52 @@
		# check_exercises.sh for Windows Rust Developers

		It is possible to run `check-exercises.sh` on Windows 10, pointing to the Windows location for you GitHub repository. This is done with the Ubunuto on Windows subsystem.


		This will redownload and build any crates needed, as they only existed in your Windows Rust.

Added pig-latin exercise and Windows Testing Notes #319

Added pig-latin exercise and Windows Testing Notes #319

Conversation

sacherjj commented Jul 17, 2017 • edited Loading

petertseng commented Jul 17, 2017 • edited Loading

petertseng commented Jul 17, 2017

sacherjj commented Jul 17, 2017 • edited Loading

sacherjj commented Jul 17, 2017 • edited Loading

petertseng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sacherjj commented Jul 19, 2017 • edited Loading

ijanos left a comment

Choose a reason for hiding this comment

sacherjj commented Jul 19, 2017

petertseng commented Jul 19, 2017 • edited Loading

petertseng commented Jul 19, 2017

petertseng commented Jul 21, 2017

petertseng commented Jul 21, 2017

sacherjj commented Jul 21, 2017

petertseng commented Jul 21, 2017

sacherjj commented Jul 17, 2017 •

edited

Loading

petertseng commented Jul 17, 2017 •

edited

Loading

sacherjj commented Jul 17, 2017 •

edited

Loading

sacherjj commented Jul 17, 2017 •

edited

Loading

sacherjj commented Jul 19, 2017 •

edited

Loading

petertseng commented Jul 19, 2017 •

edited

Loading