Skip to content

Remind learners to remove Skip properties from tests (in Leap)#726

Merged
ErikSchierboom merged 7 commits intoexercism:masterfrom
j2jensen:master
Nov 24, 2018
Merged

Remind learners to remove Skip properties from tests (in Leap)#726
ErikSchierboom merged 7 commits intoexercism:masterfrom
j2jensen:master

Conversation

@j2jensen
Copy link
Copy Markdown
Contributor

@j2jensen j2jensen commented Nov 9, 2018

Also combined the two "Notes" sections under one header.

Copy link
Copy Markdown
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Thanks a lot for this PR. Directly editing the README is unfortunately not the way to go, as the README files are re-generated automatically from time to time. That said, I think the message itself is actually incredibly useful, and I think we could just include it in every single README.

To update the general README, you should edit the config/exercise_readme.go.tmpl file. Once this file has been edited, you can the re-generate the documentation using the update-docs.sh command in the root (we still have to create a Windows version of that script).

What do you think about this approach?

If you feel this is a good idea, you could then also remove the last bit in the template on "Submitting Incomplete Solutions", which is still a leftover from the previous version.

Comment thread exercises/leap/README.md Outdated
@j2jensen
Copy link
Copy Markdown
Contributor Author

j2jensen commented Nov 9, 2018

@ErikSchierboom That makes sense, and I'll try to get to that soon.

@j2jensen
Copy link
Copy Markdown
Contributor Author

@ErikSchierboom I've updated the template file, but unfortunately lack a linux machine to run the shell script from. I tried using git bash, and got the following error from the fetch-configlet command:

gzip: stdin: not in gzip format
tar: Child returned status 1
tar: Error is not recoverable: exiting now

Not sure what to do with the pull request at this point.

Copy link
Copy Markdown
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left two small notes.

Regarding the re-generating of the documentation, I'll try and add a script that can run on Windows this weekend.

Comment thread config/exercise_readme.go.tmpl Outdated
Comment thread config/exercise_readme.go.tmpl Outdated
@ErikSchierboom
Copy link
Copy Markdown
Member

@j2jensen PR #730 has just been merged, which adds a update-docs.ps1 script with which you can easily regenerate the docs on Windows.

@turnkey-commerce
Copy link
Copy Markdown

This is a good PR, please complete this one as many beginners are tripping up on this issue. Just as tip for another method to do Linux scripts if you have Windows 10 is to install the Windows Subsystem for Linux and I often use it for running bash scripts.

https://docs.microsoft.com/en-us/windows/wsl/install-win10

@ErikSchierboom
Copy link
Copy Markdown
Member

@j2jensen Are you still interested in working on this PR?

Also combined the two "Notes" sections under one header.
@j2jensen
Copy link
Copy Markdown
Contributor Author

Yes, I'm working on it. Sorry it's been a while.

@j2jensen j2jensen force-pushed the master branch 2 times, most recently from 2720721 to 443cdff Compare November 24, 2018 04:23
ErikSchierboom and others added 2 commits November 23, 2018 21:23
@j2jensen
Copy link
Copy Markdown
Contributor Author

All right, I think I've got it. @ErikSchierboom can you let me know if there are any other changes I need to make?

@ErikSchierboom ErikSchierboom merged commit cb19cd5 into exercism:master Nov 24, 2018
@ErikSchierboom
Copy link
Copy Markdown
Member

This is looking great! I think this will really help a lot of students. Thanks so much for doing this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants