Remove unused local variables

[simenheg]

After a similar cleanup in ticket #5534, I discovered a bunch of other local variables that were assigned to but never used.

[Boris Bobrov]

i have some concerns about 5535. Although the variables are indeed not used, sometimes there is no sense in leaving the function call/class instantiation too. For example, in mediagoblin/db/migrations.py:def federation_graveyard(db) you remove metadata = MetaData(bind=db.bind) and just leave MetaData(...). I don't see any reason to call MetaData() here.

The same for config = pluginapi. ...

Some of these statements should be removed completely. By removing just the variables we actually lose evidence of how bad we are :)

[simenheg]

Thanks for your comments, breton. I've attached a new patch where I'm more ruthless about what I rip out.

I'm still not sure about whether Table(...) statements can have side effects (I don't known SQLAlchemy very well), but from the comment at lines 312-313 in test_sql_migrations.py it seems like it. Could that be the case?

[Boris Bobrov]

So, i tried to review this patch several times, but i don't understand some of the things that are being or not being removed. What does inspect_table do, for example, and what happens if we don't call it?

Could someone please have a look at the patch and confirm that it is good? Sqlalchemy is magic for me :(

[Ben Sturmfels]

Hi simenheg,

Thanks very much for the patch, and sorry we didn't get to it at the time. Unfortunately these sort of cleanups can be difficult to merge because there's a reasonable chance something might get broken along the way, so requiring very careful review and testing. Also, the patch has aged a bit now, so no longer merges cleanly on current master.

I'm going to try work through some of these unused variables as well as unused import using pyflakes and try to get them cleaned up in time.

Thanks again. Closing this ticket.

Ben