openhatch / oh-mainline

The code that runs openhatch.org
http://openhatch.org
GNU Affero General Public License v3.0
242 stars 310 forks source link

many clarity and punctuation fixes; rewrote some sections for clarity #1771

Closed li3n3 closed 9 years ago

li3n3 commented 9 years ago

Resolves https://github.com/openhatch/oh-mainline/issues/1754

"Present working directory" strikes me as pretty close to "print working directory," which is only addressed in windows-setup in the missions. As such, it seems perhaps clearer to just skip that terminology for now.

It may be better to use the kbd or <kbd class="kbd"> style in some sections where that's currently not the case; some sections do interesting stuff when viewed in an editor with syntax highlighting (e.g. swapping formatting). Still true with the e.g. patch -pnum < patchfile.patch line, since the less-than caret can get flagged as half a <tag>.

Would be just as happy to swap all the bolded code/bash lines out for kbd formatted stuff instead — there's more opportunity for it here (though I'm not sure of the precise extent), and there's an unusual (for the missions) usage of <tt> in one file (and just that file).

codecov-io commented 9 years ago

Current coverage is 78.60%

Merging #1771 into master will not affect coverage as of a814d60

@@            master   #1771   diff @@
======================================
  Files           70      70       
  Stmts         5931    5931       
  Branches         0       0       
  Methods          0       0       
======================================
  Hit           4662    4662       
  Partial          0       0       
  Missed        1269    1269       

Review entire Coverage Diff as of a814d60


Uncovered Suggestions

  1. +0.51% via ...mplatetags/search.py#49...78
  2. +0.35% via ...mplatetags/search.py#83...102
  3. +0.31% via ...file/view_helpers.py#186...203
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

ehashman commented 9 years ago

"Present working directory" strikes me as pretty close to "print working directory," which is only addressed in windows-setup in the missions. As such, it seems perhaps clearer to just skip that terminology for now.

Agree that this is confusing. Usually the phrase "current working directory" or CWD is used, where pwd is used for printing.

Would be just as happy to swap all the bolded code/bash lines out for kbd formatted stuff instead — there's more opportunity for it here (though I'm not sure of the precise extent), and there's an unusual (for the missions) usage of <tt> in one file (and just that file).

TIL about the kbd tag! That must be new. I wonder about that tt usage... at one point I think we were looking to swap out tt tags for code tags (can't find the issue right now).

Will give this a review now. Thanks for the rebase!

ehashman commented 9 years ago

I left a few comments but otherwise this looks good to me. Can you let me know if you want to work on those things or if I should just merge as is? I also should do a test run-through of the mission, just to be safe.

li3n3 commented 9 years ago

I absolutely want to work on the things! :D It's a rabbit hole, but there are so many fun rabbits.

li3n3 commented 9 years ago

Okay! I think that's it for this PR.

ehashman commented 9 years ago

LGTM! A huge thank you to @li3n3 for your fabulous contributions here.

li3n3 commented 9 years ago

It was such a great experience — thanks so much for the help, @ehashman. I look forward to doing more!