codeunion / feedback-requests-web-fundamentals

Repository for storing feedback requests from students in CodeUnion's Fundamentals of Web Development workshop.
1 stars 1 forks source link

Feedback please #477

Closed Alex1sz closed 10 years ago

Alex1sz commented 10 years ago
  1. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/count_in_list.rb I was wondering if this is well written or if there is a way to improve it?
  2. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/longest_string.rb Is it okay to do with simple return statement as so?
  3. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/sum.rb Is this the best way to write the sum method?
  4. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_pyramid.rb General feedback wanted, is this a logical/good way of writing the print_pyramid method?
  5. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_triangle.rb I can definitely remove line 10. Any other ways to improve?
  6. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/word_count.rb Basically same question as on # 2. Is it okay to have a one line method that starts with a return statement?
jcdavison commented 10 years ago

Updating my comment here, actually, you are moving the files from the week-1 directory up a level in the directory structure. Is there a reason that you were doing that? Otherwise, it would be great if you could keep the files in the original directories as it makes it easier to organize and for us giving you feedback on your code to have prior understanding of your code's file structure.

What are your thoughts on this?

Alex1sz commented 10 years ago

Yeah, something loopy happened there. I just did a new commit and pushed it "3rd commit". My second commit initially went haywire, i had to remove an index lock and then after that I was able to push but apparently it pushed something else. I just did a 3rd commit that looks like it pushed the changes, and files properly.

On Wed, Oct 8, 2014 at 11:32 AM, John Davison notifications@github.com wrote:

Something about how you are committing code, you actually did something somewhere to delete the code you wrote for sum.rb, if you look at https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/week-01/sum.rb You can see that the file is empty. Maybe think about what you did to arrive at this state?

— Reply to this email directly or view it on GitHub https://github.com/codeunion/feedback-requests-web-fundamentals/issues/477#issuecomment-58404705 .

jcdavison commented 10 years ago

In general, your code is moving along well. I was a little nervous that it seemed like you were having some development environment issues and we weren't hearing any feedback from you, but I'm glad to see you committing. Git is kind of tricky tool to build a model of understanding around.

In general, It would be better if you could make smaller commits, ie, one commit per feature and if you consider a method a feature, that would mean that you could be submitting one commit that links to one set of code per a file.

Above, you pasted in the below

1. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/count_in_list.rb
I was wondering if this is well written or if there is a way to improve it?

2. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/longest_string.rb
Is it okay to do with simple return statement as so?

3. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/sum.rb
Is this the best way to write the sum method?

4. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_pyramid.rb
General feedback wanted, is this a logical/good way of writing the print_pyramid method?

5. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_triangle.rb
I can definitely remove line 10. Any other ways to improve?

6. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/word_count.rb
Basically same question as on # 2. Is it okay to have a one line method that starts with a return statement?

It would be more effective for us to if you could post each individual question a separate issue that we could address, something like,

issue 1

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/478

issue 2

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/479

Another detail that makes code feedback more effective, you are linking to files in your request for feedback but actually linking to a commit is a much more effective pattern.

consider

issue 3

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/480

notice that in issue #3, the link I've pasted in is the actual commit of your code, not just the file.

There is a video tutorial posted on how to actually grab your commits vice the file name at the request-o-matic repo https://github.com/codeunion/request-o-matic#to-submit-a-request-for-feedback-on-your-code

Cool?

jcdavison commented 10 years ago

otherwise, :+1:

Alex1sz commented 10 years ago

Definitely will do smaller commits, and link to the commit.

On Wed, Oct 8, 2014 at 12:00 PM, John Davison notifications@github.com wrote:

In general, your code is moving along well. I was a little nervous that it seemed like you were having some development environment issues and we weren't hearing any feedback from you, but I'm glad to see you committing. Git is kind of tricky tool to build a model of understanding around.

In general, It would be better if you could make smaller commits, ie, one commit per feature and if you consider a method a feature, that would mean that you could be submitting one commit that links to one set of code per a file.

Above, you pasted in the below

  1. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/count_in_list.rb I was wondering if this is well written or if there is a way to improve it?
  2. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/longest_string.rb Is it okay to do with simple return statement as so?
  3. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/sum.rb Is this the best way to write the sum method?
  4. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_pyramid.rb General feedback wanted, is this a logical/good way of writing the print_pyramid method?
  5. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/print_triangle.rb I can definitely remove line 10. Any other ways to improve?
  6. https://github.com/Alex1sz/web-fundamentals-weekly-katas/blob/master/word_count.rb Basically same question as on # 2. Is it okay to have a one line method that starts with a return statement?

It would be more effective for us to if you could post each individual question a separate issue that we could address, something like, issue 1

478

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/478 issue 2

479

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/479

Another detail that makes code feedback more effective, you are linking to files in your request for feedback but actually linking to a commit is a much more effective pattern.

consider issue 3

480

https://github.com/codeunion/feedback-requests-web-fundamentals/issues/480

notice that in issue #3 https://github.com/codeunion/feedback-requests-web-fundamentals/issues/3, the link I've pasted in is the actual commit of your code, not just the file.

There is a video tutorial posted on how to actually grab your commits vice the file name at the request-o-matic repo

https://github.com/codeunion/request-o-matic#to-submit-a-request-for-feedback-on-your-code

Cool?

— Reply to this email directly or view it on GitHub https://github.com/codeunion/feedback-requests-web-fundamentals/issues/477#issuecomment-58409113 .