-
Notifications
You must be signed in to change notification settings - Fork 3.4k
added --preserve-case #8741
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
base: master
Are you sure you want to change the base?
added --preserve-case #8741
Conversation
943ace6 to
d06811d
Compare
57a3a59 to
90dbf35
Compare
|
Please note what I said in the thread leading up to this, in particular this comment: #7111 (comment) |
Understood, please refer to my response here. If you think this is too niche to include, or too much to review, I will maintain my fork separately for my own requirements. If the issue is the tests, I can remove those and maintain them separately. Unfortunately, property name mangling is manually inserted into the code generators, completely bypassing the Thanks for your time, please let me know soonest. |
90dbf35 to
6b4760a
Compare
|
That may be the best solution, as getting this in a mergable shape may be too much work that is not worth your time. It needs to be a minimal change. It currently hard to review because there is a lot going on. We certainly don't want to fork all of our tests for the sake of this feature. But even removing that it may still be too unwieldy. The need for these compatibility functions seems to make it worse. |
|
Understood. If there’s a more minimal way to do it I would certainly welcome feedback. The tests are not forked, new tests were added that are mirrored versions of the current tests. Of course, this means changing one leads to changing both. I will maintain a fork, and will always welcome future collaboration if it makes sense to roll this feature in at a later date. |
6b4760a to
3172c04
Compare
|
I am fixing the CI issues locally, standby for cleanup. |
3172c04 to
9939087
Compare
|
@aardappel @dbaileychess I have not tried running the CI on an Windows VM, however I think I fixed all the other issues. Let me know if you are still considering this, or if we should just close it out. Will maintain this feature on my fork. |
|
I don't think anything has changed about my evaluation(s) above. |
Summary
--preserve-case, run the matching tests, and clean up generated artifacts