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

Substitution of ToPascalCase logic with a more general one #413

Merged
merged 2 commits into from
Sep 5, 2023

Conversation

filippobottega
Copy link
Contributor

Hi,
the ToPascalCase login is not able to manage cases like these:

  • MY_FIRST_CLASS

  • MY_PROPERTY

  • ...
    For this reason I have changed the ToPascalCase logic using chviLadislav contribution found at how-can-i-convert-text-to-pascal-case.

    Please check the changes and pull the request if you like.

    Thank you and best regards,
    Filippo

@mganss
Copy link
Owner

mganss commented Sep 4, 2023

Thanks. Can you check the failing unit tests?

@filippobottega
Copy link
Contributor Author

Hi @mganss,
I have corrected some tests but TestNetex is too complex for me to let me solve the problem.
I suppose that the Generator is emitting a wrong code because some partial classes are inheriting from different classes because a number postfix on the base class. May be the Generator is not able to handle a this special case because of the new ToPascalCase function.
I have created a sample solution to understand the problem but is not possible for me to understand how to solve it in short time.
If you have any idea please drop me a line.

Thank you,
Filippo

ConsoleApp1.zip

@mganss mganss merged commit b6278be into mganss:master Sep 5, 2023
@mganss
Copy link
Owner

mganss commented Sep 5, 2023

The difference in class names revealed a name clash corner case. Should be fixed now.

@mganss mganss mentioned this pull request Sep 5, 2023
@MeikelLP
Copy link

FYI this was a breaking change for us. We had types named TRG which are now named Trg which had us to adjust several source files + hours of debugging.

Please be aware of such changes in the future.

@filippobottega
Copy link
Contributor Author

Hi @MeikelLP,
I agree with you about the fact that this change is a breaking one. The problem is the lack of a standard defining the behaviour of ToPascalCase functionality.
It might be more friendly to restore the old behaviour and implement a second way to change the names, but this will probably take more time.
I suggest changing the version number of the realease to indicate the breaking change.

Sorry for the inconvenience,
Filippo

@mganss
Copy link
Owner

mganss commented Sep 18, 2023

@MeikelLP You're right, I'm sorry. Wondering if it makes sense to release a new major version now that the horse has bolted.

@MeikelLP
Copy link

I mean it wasn't very smart on our side to just use the "latest" in the first place however I think it's smarter for us to go with the "latest" than sticking to the "legacy"

@mganss I think a major or minor (2.1) may be valid here. This also gives you the opportunity to note breaking changes (on the release page?)

mganss pushed a commit that referenced this pull request Sep 22, 2023
@mganss
Copy link
Owner

mganss commented Sep 22, 2023

I've released 2.1.908.

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.

3 participants