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

Change variable names #32

Closed
wants to merge 3 commits into from
Closed

Change variable names #32

wants to merge 3 commits into from

Conversation

pratyushprakash
Copy link

Variable names have been changes as mentioned in Issue #31

@codecov-io
Copy link

codecov-io commented Oct 10, 2016

Current coverage is 100% (diff: 100%)

Merging #32 into master will not change coverage

@@           master   #32   diff @@
===================================
  Files           5     5          
  Lines         165   165          
  Methods         0     0          
  Messages        0     0          
  Branches        0     0          
===================================
  Hits          165   165          
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update 79a2624...19cb825

@ankitaggarwal011
Copy link
Owner

Good work @pratyushprakash. There are a lot more variables and methods that can use better and more intuitive naming. Please let me know if you are still working on them and want to include more commits in this request. Thank you for your contribution.

Closes #27.

Add Google Style Python docstrings to the module, classes, and functions.
@pratyushprakash
Copy link
Author

@ankitaggarwal011 The thing is I don't understand CNN's very well so I could not understand what the methods were doing and what the variables represented

@ankitaggarwal011
Copy link
Owner

@pratyushprakash, don't worry about it. Please resolve the current conflicts with this request and I'll merge it in its current state. However, this doesn't close the respective issue, so please comment on the issue that it requires work and that your not working on it anymore such that other interested people can start working on it. Thank you for your contribution.

@ankitaggarwal011
Copy link
Owner

@pratyushprakash, please update your branch with the latest code on master branch and then add your commits, currently it's recommiting the changes that have been already merged. Thanks.

@ankitaggarwal011
Copy link
Owner

@pratyushprakash, would you please resolve the problem and we can close it.

@ankitaggarwal011 ankitaggarwal011 mentioned this pull request Oct 15, 2016
@pratyushprakash
Copy link
Author

@ankitaggarwal011 How do I remove the previous commits?

@ankitaggarwal011
Copy link
Owner

@pratyushprakash, since doc-strings had been introduced in the code, you would need to rename variables in that as well. You can either reset the recent commits, pull the latest master branch, and make your changes or start afresh in a new branch and a new pull request.

I would suggest you to start afresh, but I would request you this time to try to incorporate more changes with respect to renaming the variables. I would help you out wherever required. Thank you.

@ankitaggarwal011
Copy link
Owner

The request has been updated as #34. Closing this one.

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

Successfully merging this pull request may close these issues.

4 participants