-
-
Notifications
You must be signed in to change notification settings - Fork 920
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
refactor(internet): rename userName method to username #3130
refactor(internet): rename userName method to username #3130
Conversation
✅ Deploy Preview for fakerjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #3130 +/- ##
========================================
Coverage 99.96% 99.96%
========================================
Files 2798 2798
Lines 227793 227806 +13
Branches 955 577 -378
========================================
+ Hits 227709 227737 +28
+ Misses 84 69 -15
|
I wasn't aware the old one needs to be deprecated, I expected it to be refactor issue. |
@matthewmayer I have made the changes and deprecated the userName Its showing cannot use deprecated code. |
The error messages are correct as eslint is warning you about the usage of deprecated code. In this specific case, it is obviously ok, since you just maked this method as deprecated. We have this rule to make sure that we are no longer using the deprecated method in any of the live source code. For the test files you can use a comment to disable the check on the next line. An example would currently be in the image module test where faker/test/modules/image.spec.ts Lines 116 to 124 in c15b488
|
Issue: Unit Tests are failing at checking the deprecated tag on new As Please suggest a way around this. |
Great detective work! |
You can use the following as a workaround:
function resolvePathToMethodFile(
moduleName: string,
methodName: string,
signature: number
): string {
const dir = resolveDirToModule(moduleName);
// TODO @ST-DDT 2024-09-23: Remove this in v10
if (methodName === 'userName') {
methodName = 'userName2';
}
return resolve(dir, `${methodName}_${signature}.ts`);
} |
I noticed on both username and userName link to https://deploy-preview-3130.fakerjs.dev/api/internet.html#username (as the hash fragments are lowercased) |
For me it two different ones:
Am I missing something? |
The links from the https://deploy-preview-3130.fakerjs.dev/api/ page both go to the same place. |
I pushed a fix that should solve that issue. |
Dear @suyashgulati, Thank you for your patience and contributions so far. We wanted to provide you with a brief update. We are pleased to inform you that we have addressed all the known bugs in version 9.0. We will now take a short break and resume work around October 7, 2024, to begin merging new features for v9.1, including yours. In the meantime, we hope you have a great time, and we sincerely appreciate your ongoing support and contributions. Best regards, |
afe723e
Thanks for your contribution. |
Aims to resolve #3023
As the team has decided to user username as a convention over the current userName method name.
This PR refactors the code to use username instead.