Skip to content

Conversation

maennchen
Copy link
Contributor

Preparation for #419 (comment)

@josevalim
Copy link
Member

Sorry for being a nuisance but this is also changing the implementation of how we collect versions and so on, and we don't need to do that, do we? Can we keep that functionality as is today?

@maennchen
Copy link
Contributor Author

maennchen commented Jun 29, 2022

@josevalim I thought we have a good case for refactoring this part of the code since we're manually concating strings to build SQL. The change uses the dump command as well which supports dumping a single table.

I believe this is an improvement both in readability and the effective use of available tools.

If you do not agree, I will close this PR and start implementing the changes without touching any unrelated parts.

@josevalim
Copy link
Member

I think not using string concatenation is a great feature but I wouldn't touch how we get/generate the versions. Let's not rock the boat too much. :)

@maennchen
Copy link
Contributor Author

@josevalim Ok, then I'll just open a PR for the implementation of dump_cmd and close this one.

@maennchen maennchen closed this Jun 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants