search

alcantarar / ASB_Tutorial

Repository for 2020 American Society of Biomechanics Tutorial
MIT License
11 stars 46 forks source link

Breakout3 #20

Closed GBruening closed 4 years ago

GBruening commented 4 years ago

Addressing #3

Added requested changes that undo some of reviewer two comments.

Changed a few of reviewer 2's comments to make them look bad. Changed from int/255 to the decimal values so the code is consistant.

Split the main subscripts and solutions. We can figure out if we want this or not.

alcantarar commented 4 years ago

Thanks for working on this! I'm gonna hold off on a review until after tomorrow's meeting.

GBruening commented 4 years ago

I hate the solutions in the script files. If you want to put them there I'll let you. But if you don't want them copying and pasting like you said in another comment, then they shouldn't be there. The changes are so simple, if they can't figure it out they won't be able to figure out git.

The markdown had information on checking out to a specific branch, and I'm not sure where it went. I don't think it should be addressed here how to checkout to specific commits but should be done earlier.

This checkout method is going to be a little odd. Because they 'll need to go into their branch and checkout that branch, then pull request again.

alcantarar commented 4 years ago

I hate the solutions in the script files.

🙄

If you want to put them there I'll let you. But if you don't want them copying and pasting like you said in another comment, then they shouldn't be there. The changes are so simple, if they can't figure it out they won't be able to figure out git.

Yep, no solution files. It was simpler before and allows for easy reference because again, the point of the tutorial isn't how to debug or see if they can figure it out. We need this tutorial to be as user-friendly as possible.

The markdown had information on checking out to a specific branch, and I'm not sure where it went. I don't think it should be addressed here how to checkout to specific commits but should be done earlier.

This checkout method is going to be a little odd. Because they 'll need to go into their branch and checkout that branch, then pull request again.

We can talk tomorrow about what this breakout room is/isn't supposed to be about.

alcantarar commented 4 years ago

OK this PR is ready for review @GBruening.

post reviewer comments: breakout2_fig finished/original figure: breakout1_fig

alcantarar commented 4 years ago

I guess gary can't review this. I'll merge this because it's at least functional and we can address revisions later.

GBruening commented 4 years ago

I can review it but do that later probably, and I made this comment in the issues.

If we have the solutions in the file, it encourages them to just copy paste it or uncomment and comment things. Literally 0 investment into what they actually need to do. If they actually have to do something, they'll have a better chance of remembering it because theres more investment. The problems are not hard to fix, if you hand them everything on a plate they'll forget it all