-
-
Notifications
You must be signed in to change notification settings - Fork 300
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
Enhance setting renaming #317
Enhance setting renaming #317
Conversation
1ca325a
to
7214646
Compare
src/GUI/ReverseEngineer20/ReverseEngineer/ReplacingCandidateNamingService.cs
Show resolved
Hide resolved
Could you add some tests? |
Sure I push unit test for it |
src/GUI/UnitTests/PluralizerTest.cs
Outdated
var actResult = sut.Pluralize("OldTable"); | ||
|
||
//Assert | ||
StringAssert.Contains(expected, actResult); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why Contains?
src/GUI/UnitTests/PluralizerTest.cs
Outdated
new TableRenamer | ||
{ | ||
Name = "old_table", | ||
VariableName = "TableVariables", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we find a better name?
|
||
namespace ReverseEngineer20.ReverseEngineer | ||
{ | ||
public class InflectorPluralizer : IPluralizer | ||
{ | ||
private readonly Dictionary<string, string> _customerCharacterPluralize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_customer => _custom
.Where(x => !string.IsNullOrWhiteSpace(x.VariableName)) | ||
.ToDictionary(key => key.Name.ToPascalCase(), value => value.VariableName); | ||
|
||
_customerCharacterSingularize = customReplacers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this not always block standard renaming from working?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't known what's your meaning
I think If you want to custom name, you only want your name is equals your naming not be transfer by other function?
dc9371f
to
d679f76
Compare
d679f76
to
b4dfc22
Compare
This looks good, will just let it sit for while until the EF Core 3.1 dust settles |
Closing in favor of: #496 |
Hello I modify this #316
and I provider a better way to setting name by configure file, but this method can't not distinguish schema name