Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Peer Review of Code 4/10/2016 #6

Open
CallumChalmers29 opened this issue Oct 4, 2016 · 5 comments
Open

Peer Review of Code 4/10/2016 #6

CallumChalmers29 opened this issue Oct 4, 2016 · 5 comments

Comments

@CallumChalmers29
Copy link

Hi Luis,

Great start on your code.

What is good about your code:

  • You are using comments to describe what each line of code does which makes it easy to read

What improvements you could make:

  • You could put a mark down cell before each code cell explaining the overall goal the code in the next box achieves, this would help the code flow better and help the user understand how the boxes are linked
  • In "In [50]" you say 'split by comes' but you mean commas
  • In "In [51]" your comments "I try join the main features of a particular variable that will be my imput once the program is running" and "print whatever is clasify as my obsidian object and will display specific elements." are very vague because I don't know what "main features" or "specific elements" are, comments should be simple and specific
  • A lot of your code is practice which I would recommend you do in another notebook, the idea of Agile is to ship a working product at the end of each sprint but if the Product Owner tested this code, it would break instantly because of syntax errors, keep this in mind when submitting your milestones

With a few minor adjustments, your code could be excellent :) Keep up the good work!

Callum

@Luxo736
Copy link
Contributor

Luxo736 commented Oct 5, 2016

Hi Cal,

Thank you for your nice comments.

Could you please check if my update is working better?

I have tried to improve the points, maybe now looks more organized.

let me know.

Luis.

@CallumChalmers29
Copy link
Author

Hi Luis

Re your improvements
-You have not added any markdown cells so this is still an issue
-The spelling mistake "comes" has not been fixed
-The comments in "In [51]" were not changed
-There is still practice code in the file called Final project.ipynb which is not appropriate

New feedback
-The title "Final project.ipynb" is quite vague and should not contain spaces

Thanks
Callum

@CallumChalmers29
Copy link
Author

Hi @Luxo736 have you made these changes? We need to sort this issue by Friday

Thanks
Callum

@Luxo736
Copy link
Contributor

Luxo736 commented Oct 13, 2016

Hi Cal:
Thank you for your comments.
please check the changes and let me know what you think.

Regards.
Luis-Miguel

@CallumChalmers29
Copy link
Author

Hi Luis,

Resolved issues:
-The spelling mistake was fixed
-You also made the comes in "In [51]" more specific
-It looks like you removed all the practice code which is great

Ongoing issues
-You have added two markdown cells which is a good start but "Project: description" needs to be filled in, I think adding more markdown cells would still be useful
-I still think the title needs to be changed

New issues:
-In your second code box there is a line which just says "datarows" and another line which just says "cell_values", both accomplish nothing and can be removed to clean up your code
-"In [4]" contains incomplete code, you should comment this out so it does not interrupt the running of your programme

You have made a lot of progress which is very impressive! Keep up the great work :)

Callum

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants