swcarpentry / DEPRECATED-bc

DEPRECATED: This repository is now frozen - please see individual lesson repositories.
Other
299 stars 382 forks source link

proposed changes to the code review lesson #517

Closed lonnen closed 10 years ago

lonnen commented 10 years ago

edit: ok, this is ready for review

I have some ideas for improving novice/extras/02-review.md and would like some feedback before I start working. I propose the following:

  1. split out a lesson on creating and maintaining your own fork, to be taught before the code review lesson
  2. enhance the existing code review lesson to teach how to create pull requests between branches within one repo and across forks
  3. flush out a little bit of technique for how to review code

I'm not sure if point 1 needs to be a separate lesson or if this can be structured together as one. Point 2 supports a simplified branching model I see at some companies and expect to see at some labs where individuals all work on branches of the parent repo.

gvwilson commented 10 years ago

Our novice lessons don't introduce branching - it's too much on top of everything else for people who've never seen version control before - but the rest looks good to me.

lonnen commented 10 years ago

I had this in mind in the extras section, where 02-review.md is currently and 01-branching.md is the first lesson. Did you want it in the main novice curriculum?

gvwilson commented 10 years ago

Putting it in extras makes sense - please go ahead with your plan, and thanks.

lonnen commented 10 years ago

In progress. The new 02-forking lesson is ready for feedback.

still todo:

lonnen commented 10 years ago

ready for review + merge

gvwilson commented 10 years ago

Can the Git topic maintainers please decide whether to merge this one, and if so, resolve the conflicts and push the button?

lonnen commented 10 years ago

I took a stab at rebaseing this myself locally but it looks like 02-collab has had some work, at least, that conflicts. Since I effectively split that into two or three separate articles it will take some work to resolve conflicts and I don't have any cycles to spare. Sorry.

jiffyclub commented 10 years ago

I've manually merged and pushed this, thanks for adding the lesson.

It occurred to me while reviewing this that there don't seem to be any actual images or descriptions of the mechanics of code review on GitHub. For example that you can leave inline comments by clicking to the "Files" tab and then on lines, or leave comments on a commit by going to the commit view. Something the flesh out in the future.

wking commented 10 years ago

On Tue, Sep 30, 2014 at 09:43:21PM -0700, Matt Davis wrote:

For example that you can leave inline comments by clicking to the "Files" tab and then on lines, or leave comments on a commit by going to the commit view.

Or just link out to here 1 or the more generic 2.

lonnen commented 10 years ago

Thank you, all involved. It's good to have it landed.