Skip to content

Mark Internet#pass deprecated#1245

Merged
snuyanzin merged 4 commits intodatafaker-net:mainfrom
snuyanzin:pass
Jun 1, 2024
Merged

Mark Internet#pass deprecated#1245
snuyanzin merged 4 commits intodatafaker-net:mainfrom
snuyanzin:pass

Conversation

@snuyanzin
Copy link
Collaborator

Internet#password is very limited with its functionality and we should encourage users to use Text instead which covers much more cases

@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.03%. Comparing base (b37c566) to head (0fb1225).
Report is 147 commits behind head on main.

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

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1245      +/-   ##
============================================
- Coverage     92.35%   92.03%   -0.32%     
- Complexity     2821     3042     +221     
============================================
  Files           292      309      +17     
  Lines          5609     5987     +378     
  Branches        599      629      +30     
============================================
+ Hits           5180     5510     +330     
- Misses          275      319      +44     
- Partials        154      158       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we bundle unit tests in the library? It seems strange to document that users should check unit tests.


/**
* Consider usage of {@link net.datafaker.providers.base.Text#text} instead,
* look at net.datafaker.providers.base.TextTest#textShouldContain3RULowerCaseAnd5CustomSpecialSymbols()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RULower? Regular upper lower?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

textShouldContain3RULowerCaseAnd5CustomSpecialSymbols

means that text should contain 3 lower case letters from RU locale and 5 custom special symbols

@snuyanzin
Copy link
Collaborator Author

Do we bundle unit tests in the library?

no

It seems strange to document that users should check unit tests.

yep, there are 2 ways I see: either put example in javadoc for net.datafaker.providers.base.Text#text or in site documentation

@snuyanzin
Copy link
Collaborator Author

Ok, added to javadoc

@snuyanzin snuyanzin requested a review from kingthorin June 1, 2024 18:49
Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I think that's much more usable

@bodiam
Copy link
Contributor

bodiam commented Jul 4, 2024

I've reverted this change. Without these methods it's way to complex too create a password, and I don't see why these methods can't exists next to each other.

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.

4 participants