GothenburgBitFactory / tasklib

A Python library for interacting with taskwarrior databases.
http://tasklib.readthedocs.org/en/latest/
BSD 3-Clause "New" or "Revised" License
148 stars 28 forks source link

[fix] Follow pep8 guidelines #46

Closed lyz-code closed 6 years ago

lyz-code commented 7 years ago

Hi, I'm trying to contribute some code to interact with the history of Taskwarrior to get the active time of the tasks.

But I've seen that there were some PEP8 and other guideline warnings and I've decided to fix them.

I hope you're OK with it :)

lyz-code commented 7 years ago

@robgolding @tbabej It seems that travis is failing because it's trying to clone an old repo

git clone --recursive https://git.tasktools.org/scm/tm/task.git

Maybe it should point to https://git.tasktools.org/tbabej/task

lyz-code commented 6 years ago

@robgolding how can I rerun the tests?

robgolding commented 6 years ago

@lyz-code if you merge master into this branch Travis should re-run the build. However there are some indentation/linting styles in this PR that are a little unusual so wouldn't get merged. Most of the stuff is an improvement, though!

lyz-code commented 6 years ago

@robgolding It seem's to be passing the tests now.

Which indentation/linting is unusual? I've followed the guidelines of the python-mode vim plugin :S

Just tell me which lines are and I'll revert them so it can be merged.

Thanks for your time!

:)

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 1225677a596f1c36eb54ed15459ac9a40f0233e2 on lyz-code:fix/pep8-guidelines into on robgolding:develop.

lyz-code commented 6 years ago

Hi @robgolding, I guess you are busy (I see it in your commit frequency).

Whenever you can I'd be glad if you could tell me what do you see odd. I see you have a lot of experience and I'd like to learn :)

Furthermore, I'm also working in a feature of tasklib to parse the history and work with times natively. I'd like to know your oppinion on that soon.

Thanks for your work

robgolding commented 6 years ago

Hey @lyz-code! Sorry for being unattentive here. I'll do a code review with examples where the formatting is a bit unusual, and my preferred alternative. Note that this is personal opinion, and my interpretation of PEP8! ๐Ÿ˜„

coveralls commented 6 years ago

Coverage Status

Changes Unknown when pulling 8f4fabfeb07bd218ac24bb00d5e1c123e444bff1 on lyz-code:fix/pep8-guidelines into on robgolding:develop.

robgolding commented 6 years ago

No problem, it looks great now! And all the tests pass, so letโ€™s merge ๐Ÿ‘