Skip to content

Conversation

umangahuja1
Copy link
Contributor

Review.

I tested all the four features and they are working.
Plus updated the .gitignore file.

@sipah00
Copy link
Owner

sipah00 commented Dec 1, 2017

@umangahuja1, it is a good start, but changes in lscorecard.py are not necessary (no need to put extra () ), so remove it and for changes in score.py, I will verify soon.

@umangahuja1
Copy link
Contributor Author

Yup I saw that, but to make sure all changes are done from 2 to 3, I used ide to convert it.

@sipah00
Copy link
Owner

sipah00 commented Dec 1, 2017

@umangahuja1, for py2 to py3, check which package is not compatible and change it accordingly like you have done it in score.py , I think I have already used packages which are also compatible with py3 in other scripts best way to check, run CrickFev in py3 env and see errors and if there is no error it also compatible with py3.

@umangahuja1
Copy link
Contributor Author

Yup I ran and there are no errors. All 4 were running good.

@umangahuja1
Copy link
Contributor Author

Have you reviewed this PR.

@sipah00
Copy link
Owner

sipah00 commented Dec 9, 2017

@umangahuja1, I had reviewed it and i got errors in score.py, correct, run and then send PR.

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

Successfully merging this pull request may close these issues.

2 participants