search

arcus / Arcus_Labs_Orientation

Orientation for new Arcus Scientific Labs users
https://liascript.github.io/course/?https://raw.githubusercontent.com/arcus/Arcus_Labs_Orientation/main/arcus_orientation.md
1 stars 1 forks source link

Data contribution 2 #37

Closed nafeldman closed 8 months ago

nafeldman commented 8 months ago

Hi Education Team! Please review as your time allows. Our goal is to have these ready to be published by the end of the month.

nafeldman commented 8 months ago

thank you @pm0kjp - that's a really good point about adding alt text (especially to the gifs) we should definitely go back and do that.

allrolsen commented 8 months ago

This looks good but I realized at around line 600 that I keep repeating the same things so I wanted to stop to give you a chance to look at what I've commented so far before I keep copy/pasting. The basic things I keep repeating:

  • Using text on an image to drive understanding is suboptimal for various reasons, but I can imagine that adding text into the document itself might be a huge PITA and worth doing as a "secondary improvement", so I won't hinder you moving forward on this
  • There are lots of unintentional italics where you have 2 or more underscores on the same line of text without escaping them.

Take a look at what I've suggested (and continue looking especially for unintentional italics further on, I stopped noting them but you can see your script in situ at https://liascript.github.io/course/?https://raw.githubusercontent.com/arcus/Arcus_Labs_Orientation/data-contribution-2/data_contribution_2_data.md#1 to see what it looks like)

@pm0kjp Thank you for all of your edits and suggestions. I have gone through the text and ensured that all of the _ are properly escape. Despite this, some text is still italicizing. All the text that still does this is with the scripts. Is there someone we can reach out to at LiaScript to look at this?

rosemm commented 8 months ago

Hi @allrolsen I noticed the problem you pointed out about the escaped underscores not working within the contingent text. It's because that text is piped in as part of a script -- anything that needs to be escaped in that text has to be escaped twice (e.g. \\_) because one "layer" of the escaping happens as the text is parsed by the script, and you still need an escape present there when it comes out the other side and gets inserted into the page. I went through and put those extra escapes in for you, and I think it's all rendering correctly now --- let me know if you still see problems!

rosemm commented 8 months ago

I also saw a few typos. Do you have spell check enabled in your text editor? (https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker) I'm a terrible speller, so I rely on it a lot :) It saves me a ton of time and errors.

allrolsen commented 8 months ago

Hi @allrolsen I noticed the problem you pointed out about the escaped underscores not working within the contingent text. It's because that text is piped in as part of a script -- anything that needs to be escaped in that text has to be escaped twice (e.g. \\_) because one "layer" of the escaping happens as the text is parsed by the script, and you still need an escape present there when it comes out the other side and gets inserted into the page. I went through and put those extra escapes in for you, and I think it's all rendering correctly now --- let me know if you still see problems!

Thank you!! Nicole and I were really annoyed by this, and had no idea how to fix it.

allrolsen commented 8 months ago

I also saw a few typos. Do you have spell check enabled in your text editor? (https://marketplace.visualstudio.com/items?itemName=streetsidesoftware.code-spell-checker) I'm a terrible speller, so I rely on it a lot :) It saves me a ton of time and errors.

I did not have spell check enabled, thank you for sharing that tool. I added it to VS Code and fixed all the spelling mistakes.

allrolsen commented 8 months ago

@pm0kjp and @rosemm Thank you both again for reviewing this module, your suggestions and help are invaluable! @nafeldman and I fell it looks good and are ready to merge this branch. I am not able to do it, it says there are changes requested (believe I resolved all of them) and that there is a conflict (media/source/.DS_Store file). Could one of you merge it? Someone with an Admin role should be over to override these issues.

rosemm commented 8 months ago

I fixed the merge conflict, so it should be ready to go now. Unfortunately, it looks like github won't let me merge it without @pm0kjp specifically signing off, since she had requested changes earlier. Joy's out today, so it might be a little while before she has a chance to approve.

pm0kjp commented 8 months ago

I approve! I'm "half in" today :)