Skip to content

Conversation

@jordihs
Copy link

@jordihs jordihs commented Apr 11, 2020

Fix for issue HBX-2014.
Followed instructions given at the forum (see https://discourse.hibernate.org/t/hbm2java-uses-default-plattform-encoding-for-source-generation/3649).

@jordihs jordihs changed the title Fix for BVAL-748: Output for template generation set to UTF-8 Fix for HBX-2014: Output for template generation set to UTF-8 Apr 12, 2020
…for a column alongside those provided by the hibernate.reveng.xml file
@koentsje
Copy link
Member

Hello @jordihs, it seems to me that you are mixing concerns in these pull requests. Can you please open a pull request containing the commit for the UTF-8 issue and open a separate issue and pull request for the RevengStrategy enhancement? Also it would be good if you could provide a test that guards that this works so we don't inadvertently delete or change the implementation later.

@koentsje koentsje mentioned this pull request Apr 14, 2020
@jordihs
Copy link
Author

jordihs commented Apr 14, 2020

I messed up with the pull requests, it seems. I intended each one to be specific to its issue. I'm sorry, I am not used to this kind of workflow. I'll try and do as you say when I get some time to do it.

@koentsje
Copy link
Member

koentsje commented Apr 14, 2020

Maybe just close them and create new ones... Try to limit yourself to one commit per pull request, that makes the review process easier too
The best thing is to create a topic branch (i.e. HBX-2014) in which you implement the UTF-8 issue and another branch (HBX-2015) for the reveng issue. Make sure both branches are branched off from the master branch and only contain the commit relevant to the issue. The rebase/merge will be done by me 😉

@jordihs
Copy link
Author

jordihs commented Apr 14, 2020

OK, will do. Thanks for the pointers.

@koentsje koentsje deleted the branch hibernate:master April 11, 2022 15:42
@koentsje koentsje closed this Apr 11, 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