Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Table casing #248

Closed
wants to merge 2 commits into from
Closed

Table casing #248

wants to merge 2 commits into from

Conversation

cocytus
Copy link

@cocytus cocytus commented Sep 14, 2022

Add option to use lowercase table names.

@cocytus
Copy link
Author

cocytus commented Sep 14, 2022

Fixes #245

@erikbra
Copy link
Owner

erikbra commented Sep 14, 2022

Thanks a lot for your contribution, @cocytus !!
This is of course one way of solving this issue. I'd just like to have your opinion on my comments here #245 (comment) before I merge this. I try to be a bit conservative with adding new command-line options, as there are already very many, and if this is really a workaround for a bug(ish), I want to consider other options as well.

What do you think of the suggestion of not enclosing the table names in quotes when creating the grate tables for PostgreSQL? Would that work too?

@cocytus
Copy link
Author

cocytus commented Sep 21, 2022

Hei,

Please take a look at the updated PR code. This "works on my machine" :-) I don't have docker, so only a few of the tests actually works.

@cocytus
Copy link
Author

cocytus commented Oct 1, 2022

About the failed tests: Unsure why that happens? On first glanse it seems unrelated to my PR. I rebased on latest main, hope you can retrigger the workflows.

@erikbra
Copy link
Owner

erikbra commented Oct 1, 2022

I'm not sure why the tests fail either, I'll try to run the tests on "main" to see if they fail too

@erikbra
Copy link
Owner

erikbra commented Jun 3, 2023

Abandoning this in favor of #256

REALLY sorry for the long time it has taken me to get around to do the last 20% of this, @cocytus. I didn't mean to disrespect your efforts here, and in addition, refuse to finish my own work doing the same in a different way.

I hope I haven't scared you away from creating PRs to grate (although I respect and understand if I have 🤕 ).

Anyways, the feature should be on its way out in the next grate release. Hope to release it in the next few days.

@erikbra erikbra closed this Jun 3, 2023
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