draft: migrate disease provider under healthcare providers (#1137)

[filipowm]

Hi,

I started migrating Disease provider under healthcare, however I'm not sure if this wouldn't break backward compatibility, because:

1. Disease is moved from BaseProviders to HealthcareProviders
2. disease.yml was changed to be prepended keys with healthcare prefix.

What are your thoughts on this? Is this approach safe enough to move on?

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.02%. Comparing base (b37c566) to head (ae00445). Report is 141 commits behind head on main.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1188      +/-   ##
============================================
- Coverage     92.35%   92.02%   -0.33%     
- Complexity     2821     3037     +216     
============================================
  Files           292      308      +16     
  Lines          5609     5981     +372     
  Branches        599      629      +30     
============================================
+ Hits           5180     5504     +324     
- Misses          275      319      +44     
- Partials        154      158       +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[bodiam]

To me, though I only had a quick look and didn't test it myself, this looks very good. I don't think the API is broken (maybe we should have a library API tester?), since the decease only moved, and the public API seems quite similar, so on a quick glance, it seems the API hasn't been broken.

[kingthorin]

Completely changing the location of methods is API breaking. Users have to re-code things to make them work again. While it is minimal effort to adapt it's still a breaking change.

[bodiam]

Completely changing the location of methods is API breaking. Users

I might be missing something, but I don't see it? They will still do faker.disease().icd10(), why would any user care if the disease() is part of the BaseProviders or HealthcareProviders, it's still the same API call.

[kingthorin]

I'll have to pull the branch and play with it.

If, as you've said, the methods are still accessed the same way then I totally agree it's non-breaking.

I took bullet one from the PR description to mean that access had changed.

[kingthorin]

🤦‍♂️ I'm being brain dead. The unit tests would have been different if there were API changes. I know they did move but the faker usage is the same. So ignore me 😔

[bodiam]

The tests are failing, but not because of your changes, there still seems to be something broken in our internal works: java.lang.RuntimeException: Unable to resolve #{Internet.password '5','8','true','true'} directive for FakerContext FakerContext{, locale=SingletonLocale{locale=test}, randomService=randomService}.

[kingthorin]

I haven't looked but is this PR branched after that fix or does it need rebasing?

[filipowm]

I haven't looked but is this PR branched after that fix or does it need rebasing?

@kingthorin I branched out on saturday, so it might require rebasing. I will do that in the evening.

[kingthorin]

datafaker-net/datafaker#1180 was merged "3 Days ago" (that's all the web UI shows). So hard to know.

[bodiam]

Hi all, what should we do with this PR?

[snuyanzin]

I guess addressing of this comment is still in progress https://github.com/datafaker-net/datafaker/pull/1188#discussion_r1590102934 @filipowm do you need help with it?