-
-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Apply semantic line breaks #201
Conversation
Massive thanks @tobyhodges - this is exactly what we needed and definitely on the right track. |
Wow, thanks so much for taking this on! It is looking good so far. My only comment is that we haven't yet agreed as maintainers what maximum line length we want to aim for in source files. SemBr recommends 80 characters, but I think that might be too restrictive for us. I would suggest somewhere around 100 characters, and it looks like you are loosely hitting that target with your changes. |
Thanks for the feedback, @bielsnohr - I was indeed aiming for 100 characters max, based on the title of #199, but of course a few lines will go over where a break is either impossible or I judge it would make the file less readable. |
@carpentries-incubator/python-intermediate-rsd-maintainers I have worked through all of the episodes now and hope to get through the files in If you can reply to that issue with updated wording, I am happy to add that into my changes for the episode in this PR. If you would prefer that I leave the text of that episode as-is in this branch, I will apply line breaks only. Please let me know how you would like me to proceed. |
Thanks @steve-crouch for resolving #205 I have now pushed the sembr-ed version of episode 42 (incorporating Steve's suggested wording) and this is finally ready for review and merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again @tobyhodges for this extensive amount of work on re-formatting the course files. I started reviewing the actual source files, but quickly realised that would be far too arduous for the entire course. Therefore, I switched to just making sure that the rendering of the website looked roughly correct. I have done that for all pages and am satisfied the coures website renders as it did before these changes, which is exactly what we want.
I have requested changes on the one file that I looked at in depth. I imagine that there would be similar changes for the other files, but I am adopting the approach that we can make those adjustments incrementally at the time we are making other modifications to the material.
I want this PR to get through as quick as possible because it is invariably blocking other concurrent work from happening. I myself have held off making some changes because I knew that they would cause merge conflicts with whatever was done here.
@anenadic and @steve-crouch could you give a quick yay/nay whether you are satisfied with me being the only maintainer to okay these changes and then merge?
Co-authored-by: Matthew <matthew.bluteau@gmail.com>
After verbal confirmation from @anenadic in a maintainer meeting, I am going to proceed and merge this. Thanks again @tobyhodges for the great work on this. |
You beat me to it @bielsnohr. Many thanks to @tobyhodges for this. |
When completed, this will fix #199
It is going to take me a while, but I am breaking up and reformatting lines in the source .md files in an attempt to follow the specification for semantic line breaks.
Because this will end up being a very large PR, I'm opening it now with only
index.md
and first episode edited so far. Can I get some feedback from the maintainers and confirmation that I am on the right track please, before I continue?I can't promise that the whole lesson will be done quickly - I am going to be doing this in spare time - but I'll get there eventually!