search

pytroll / pytroll-pps-runner

Pytroll runner for PPS
GNU General Public License v3.0
1 stars 8 forks source link

Fix version and package naming #13

Closed adybbroe closed 3 years ago

adybbroe commented 3 years ago

Change package name to nwcsafpps_runner, fix for Python3 and git versioning.

ghost commented 3 years ago

Congratulations :tada:. DeepCode analyzed your code in 2.983 seconds and we found no issues. Enjoy a moment of no bugs :sunny:.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

adybbroe commented 3 years ago

@mraspaud and @TAlonglong Many thanks for the useful comments. Code should be slightly more readable now.

adybbroe commented 3 years ago

And, by the way, I looked at the Deep Code bot's comments and found it partly useful only. Fixed one issue. But line numbers doesn't point correctly...

adybbroe commented 3 years ago

Thanks for your reviewing @mraspaud and @TAlonglong , do you think it looks better and ok now?

mraspaud commented 3 years ago

@adybbroe I still think the codefactor items should be resolved

adybbroe commented 3 years ago

Thanks @mraspaud ! Overlooked those. I have fixed all of them but 2 I believe. One I have asked your advice on, the other is the FIXME! which I don't have an answer to yet.

adybbroe commented 3 years ago

Okay, now down to 3 issues. The two I mentioned above plus a third on "Complex code" on line 0 in utils.py No idea atm what to do about that...

mraspaud commented 3 years ago

Complex code can be helps with refactoring

adybbroe commented 3 years ago

Complex code can be helps with refactoring

Ok, but there was no hint. I can certainly look over the complexity and refactor, but not in this PR!

adybbroe commented 3 years ago

Ok to merge @mraspaud and @pnuu ?

mraspaud commented 3 years ago

I don't see any tests, but I guess it's the rule in this package? If you are ok with that, then you can merge imo.

adybbroe commented 3 years ago

I don't see any tests, but I guess it's the rule in this package? If you are ok with that, then you can merge imo.

Yes, it is unfortunately the rule in this package. I can make an issue on that