michelebossa / HA-Scheduler

Scheduler Addon Home Hassistant
28 stars 2 forks source link

Improve code quality and change code #8

Closed KTibow closed 4 years ago

KTibow commented 4 years ago

Look at the files changed.

michelebossa commented 4 years ago

Hi KTibow, Thanks a lot for you help this is my first experiences on developing in python/flask and it is very appreciated i love the Open Source philosophy.

I see you are changed a lot of single quotes to double i think it is some best practice rules on python. ( I have a lot to learn )

I'm checking and testing you branch at moment sounds good. (Thanks to fix my grammar English errors )

If you are finished to apply the modify i'll merge to master.

Thanks.

KTibow commented 4 years ago

I just used black, a popular python formatter, to keep the code formatted. You can use an online playground if you don't want to install it.

KTibow commented 4 years ago

Does it still look good for you @michelebossa?

michelebossa commented 4 years ago

Hi, I'm in holidays this week without my laptop and with a lot of connection issue.( best situation to disconnect from the stress of our life ) Monday I'll try some check. Thanks

michelebossa commented 4 years ago

Hi, Today i have started some tests and there are some parts to changes from my point of view:

With your changes about the information of status we have an alignment issue due the different length of text on these part i think it is better to restore the checkbox (It is more clean for me)

Can you shorter the text with "Add" and "Remove"?

image

The deletion of an entity now getting an error during the rendering of result page. can you check?

If you need more information's i can try to help you.

Thanks.

KTibow commented 4 years ago

Okay, I updated that. Try uninstalling, re-adding, and re-installing. And by

deletion of an entity

do you mean removing one entity from a schedule, or a whole schedule?

michelebossa commented 4 years ago

Delete whole schedule.

image

KTibow commented 4 years ago

Odd... I'll look in to that once I have time (I'm about to be busy for a couple of hours).

KTibow commented 4 years ago

I've moved on to a scheduler component + card. Reopen + merge if you want.