pep8_fixes #116
pep8_fixes #116
Conversation
|
I'll take a look in the morning. Git diff isn't telling me anything useful, because it thinks you rewrote the whole file. So I'll have to be awake for this (oh no!) The couple changes I did notice look good so far though. It's kind of hilarious that even after fixing 200+ issues, |
|
From what I saw this almost certainly gets merged. I'll want to try and fix some Javascript style stuff as well, just to merge all of it at once, but that's on my head. |
|
To make sure though, you're:
Dolphin-emu had a really bad time with that last part, so I'd like to get ahead of it before it inevitably needs to happen. |
I just looked at the diff now and realized what I did, it was the line-endings that were changed (I only really changed the PEP8 issue parts). I think the original files were using CRLF and when I loaded them in PyCharm it replaces them with LF (my default settings on Ubuntu). When I went to commit, I noticed it said I was using CRLF and I didn't realize why so I changed them back. I think PyCharm was trying to do the right thing by converting them back when I committed them but I overrode it without thinking about it (late night coding, my mistake). I can change the line-endings back to CRLF if you want?
If you think that's appropriate, sure. I didn't really add much to the project so I'm ok if you don't want to list me there. I'm just trying to get more practice using git and GitHub so I win either way!
No problem at all, I don't want to cause any issues with the licence because of my style commits! |
I'm not sure you are correct there, I think https://codeclimate.com/github/the-zebulan/p2p-project/py_src/base.py It shows 114 issues but 113 are PEP8 style issues which I fixed in the If I understand CodeClimate correctly (this is my first time using it so I could be wrong!), I think most of the Python files style issues are fixed. So, I don't think you messed up too bad! |
|
I just went through the files in the
In my PR, all of the files (except two) now have the line-endings set as
Since the line-endings don't seem to be consistent across the files in
Let me know if you think I should make any modifications to this PR! |
Yeah, I ran into that bug before. Should have thought of that before I sounded silly.
At some point I had Sublime (or maybe git) translating these to CRLF for me, but I've had to reimage my computers a few times since then. Probably this is where the inconsistencies occurred. If you changed that back to CRLF I'd say thank you. I occasionally need to work on Windows (yuck), so it's nice for it to be consistently that way. |
|
Also, feel free to ignore the red light from Travis on the most recent test. It looks like |
|
Ok, quick question. Since some of the files had
What do you think? |
|
Just switch it all. I can deal with it on my end fairly easily. |
|
Ok, I'm on it. |
|
Looks like |
Current coverage is 71.65% (diff: 85.47%)@@ develop #116 diff @@
==========================================
Files 15 15
Lines 1492 1496 +4
Methods 0 0
Messages 0 0
Branches 240 241 +1
==========================================
+ Hits 1068 1072 +4
- Misses 390 391 +1
+ Partials 34 33 -1
|
|
Might as well leave the text files alone. I only really want to focus on the Python stuff for the moment. |
|
That last commit I pushed changed the line-endings in all of the Python files in |
|
You're now merged. I'm going to make a quick update to the author section. Would you prefer your GitHub handle, or a name? |
|
Awesome! I'm glad you approved of the changes. Also, I'm fine with you just using my GitHub username for the author section. Thanks again for your time and the merge! |
|
No problem. I'd love it if you stuck around and helped more, not that I would demand it. There's a lot of code which could probably be cleaned up, and I'm going to have to work full time on |
|
The only issue is that I don't have a lot of experience with any other language than Python (a little bit of JavaScript too, I guess) and I haven't really worked on any big projects yet either. I'm still learning about programming in general so I don't know if I'd be much help (your code seems fairly advanced compared to anything I usually write!). |
|
Would it help if I gave a specific goal? As for the other languages, feel free to leave that to me. ON some of them I don't know much of what I'm doing either. Golang especially. |
|
Ya, if you could point me in the right direction I could try and work towards goals. I'm trying to gain programming experience so any opportunity to practice/learn seems like a good idea to me! |
|
Okay.
As things stand, each `base_connection` is storing the most recent characters it received in a list. This is *horribly* inefficient.
I would be very happy if you could refactor this to store this as `bytes` instead. I don't think it will be too crazy, since it's only used in that class and a little in its children, but there will likely be lots of bugs in the early part.
I'll open a separate issue so that we don't pollute a closed one here.
…On December 1, 2016 1:42:14 AM EST, the-zebulan ***@***.***> wrote:
Ya, if you could point me in the right direction I could try and work
towards goals. I'm trying to gain programming experience so any
opportunity to practice/learn seems like a good idea to me!
--
You are receiving this because you modified the open/close state.
Reply to this email directly or view it on GitHub:
#116 (comment)
|
I fixed the PEP8 issues in all of the files in
py_srcexcept:chord.py(as discussed in the previous PR)py_src/test_utils.py. I'm sure it could be split up like the other strings but with the various punctuation in the string I didn't want to mess up the command for the windows batch file (and I don't have a Windows computer to test the command).I've been pushing these commits to my fork and checking the results on CodeClimate. Currently, the score is up to 3.0 GPA from 2.81 GPA.
Let me know what you think!