Remove public methods from `IdNumber` that are not intended to be directly called about datafaker HOT 8 CLOSED

yes that probably makes sense

why it was done in current way - probably nobody knows since it was done in javafaker first and those contributors are not active in both communities...

I can submit a minimal pull request to demonstrate this approach.

yes,go ahead

The only thing to note since they are public for a long time then first need to mark as deprecated and then remove later

from datafaker.

@asolntsev may be for future: I would suggest squashing commits while merging since it will simplify navigation through commits history
All the highlights could be done in commit message

from datafaker.

If that's the intent then I'm in agreement. I haven't been working on the project long enough to know.

from datafaker.

Makes sense. I don't think we have a super clear agreement about what is our public api vs our more internal. What would you propose to limit the visibility of these methods? Make them package protected? Or something else?

from datafaker.

I think we don't need these methods at all.
What I suggest:

1. Remove public methods from IdNumber class:
   • public String validMacedonianPersonalCode()
   • public String invalidMacedonianPersonalCode()
   • etc.
2. Load all classes like MacedonianIdNumber as service providers (a built-in mechanism in Java). Every provider knows what country it's responsible for.

I can submit a minimal pull request to demonstrate this approach.

from datafaker.

@snuyanzin Thanks. Yes, in this case I should squash all commits.
But generally it's not always a good idea. Independent changes should stay in separate commits.

from datafaker.

yep, in generic case it depends on the context

from datafaker.

For my two cents it's the overall contribution that matters to the project not the individual bits of work/correction. So I'm in favour of squash and merge or using amending commits.

from datafaker.