CodeYourFuture / syllabus

Syllabus Website for CodeYourFuture
https://syllabus.codeyourfuture.io
153 stars 124 forks source link

Updated PD JS3 W1 #672

Closed esma-g closed 1 year ago

esma-g commented 1 year ago

What does this change?

Module: JS3 Week(s): 1

Checklist

Description

Updated PD JS3 W1

Who needs to know about this?

@kfklein15 @CodeYourFuture/syllabus-team

Rendered Pages

esma-g commented 1 year ago

@illicitonion I have a question. If I merge this PR now without signing off and committing your suggestions, will I have ignored those suggestions?

If I sign them off now, your review will become stale and I'll need another review before being able to merge. Help please :)

illicitonion commented 1 year ago

If I merge this PR now without signing off and committing your suggestions, will I have ignored those suggestions?

Yes, those suggestions would be ignored.

In general it's nice to fold in the feedback before merging, but if stuff is super urgent you can merge ignoring the feedback and follow up later.

If I sign them off now, your review will become stale and I'll need another review before being able to merge. Help please :)

First thing - I'm very happy to re-review (and generally if the changes are relatively small, same day :)) so don't worry about that part at all! Would definitely rather we work quickly together to get things into a good shape than be scared of having to wait for a re-review.

In general I tend to configure repos so that new changes don't cancel out already given approvals; I assume that's not the way the syllabus repo is configured for a reason (though I will follow up with folks about that), so for now I'd say: unless there's a lot of urgency, let's push some changes to fix up the suggestions and I can just re-review quickly :)

esma-g commented 1 year ago

If I merge this PR now without signing off and committing your suggestions, will I have ignored those suggestions?

Yes, those suggestions would be ignored.

In general it's nice to fold in the feedback before merging, but if stuff is super urgent you can merge ignoring the feedback and follow up later.

If I sign them off now, your review will become stale and I'll need another review before being able to merge. Help please :)

First thing - I'm very happy to re-review (and generally if the changes are relatively small, same day :)) so don't worry about that part at all! Would definitely rather we work quickly together to get things into a good shape than be scared of having to wait for a re-review.

In general I tend to configure repos so that new changes don't cancel out already given approvals; I assume that's not the way the syllabus repo is configured for a reason (though I will follow up with folks about that), so for now I'd say: unless there's a lot of urgency, let's push some changes to fix up the suggestions and I can just re-review quickly :)

Understood, thanks!