2 new commits added
rebased
command.current(config) is just for displaying to the users, right ?
command.current(config)
Yes indeed, I used it for debugging, not sure if it's worth keeping. What do you think?
something like: "Current alembic revision id is at: " . The first one isn't required though.
Cool, let's do that then yes. Note that I don't do the print myself, so I can just print "Current alembic..." and then call the method that does the print for us.
Rest seems fine to me :thumbsup:
Probably "i.e. the folder where the revisions are stored, relative to the loaction of alembic.ini"?
alembic.ini
Instead of keeping this inside the files directory, and copying it, can we just keep it in . and name it alembic.ini.example? The dev can then rename it to alembic.ini.
files
.
alembic.ini.example
Does this work for you? I could get it working only with pagure.lib.model.BASE.metadata.
pagure.lib.model.BASE.metadata
@cep seems right :/
We should give example here or for development purpose we can tell them to point to pagure/alembic
pagure/alembic
I like this :)
Regarding alembic we should put a hacking part where it tells people what should they do. It's just a suggestion , this might benefit new contributors. :smile:
alembic
I'd rather keep it in files regardless of how we name it, to keep the top level directory somewhat "clean" :) So we'll have to move it anyway making the name change less interesting
@cep unless they specify the full path, which is what is in the default alembic.ini
Hm, it did work for me, which part didn't work for you? Stamping the DB or doing an upgrade?
Oh. In that case, files/alembic.ini should be fine; one less thing (renaming) to do :)
files/alembic.ini
@pingou I meant that we should rephrase this sentence to "i.e. the folder where the revisions are stored, relative to the loaction of alembic.ini", because it's a little confusing right now :)
Revisions didn't work. It failed with AttributeError: type object 'Base' has no attribute 'sorted_tables'
AttributeError: type object 'Base' has no attribute 'sorted_tables'
Alright, let's fix this then :)
3 new commits added
Alright folks, many thanks for the review and inputs, I think I addressed them all :)
Another round of review is welcome :)
:thumbsup:
LGTM :thumbsup:
@cep if you have a minute :)
Alright, let's rebase and merge then :)
Pull-Request has been merged by pingou
I thought I had fixed that :(