django / django-localflavor

Country-specific Django helpers, formerly of contrib fame
https://django-localflavor.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
813 stars 289 forks source link

Update prospector #412

Closed benkonrath closed 3 years ago

benkonrath commented 4 years ago

I'll be slowly working on this as I have extra time.

benkonrath commented 3 years ago

@claudep Hi Claude, I think the changes here are ok but it would be nice if you could have a quick scan to double check this. Thanks!

benkonrath commented 3 years ago

Yeah, just the one bare except that you pointed out. It's fixed now. Thanks for the review.

benkonrath commented 3 years ago

I'm also fine with fixing the no-else-return and no-else-raise problems that are reported when these pylint options aren't disabled. I just wasn't sure how "wrong" or "bad" the "if return/else return" pattern really is. Do you have an opinion?

claudep commented 3 years ago

I've just been recently confronted to that. I don't think it matters so much. Of course, having else after a return is unnecessary technically speaking, but in another view, having the else balancing the if does make some semantically sense to me. So do what you like.

benkonrath commented 3 years ago

I also think the else makes sense semantically - although in new projects I usually just accept the pylint default and don't add the else when returning in a chain of if statements. I'll leave it as is since doesn't really cause too much harm. Thanks for your input on this.