Skip to content
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(date): rename abbr to abbreviated #2068

Merged
merged 7 commits into from
Apr 18, 2023

Conversation

Shinigami92
Copy link
Member

reference #2061 (comment)

@Shinigami92 Shinigami92 added c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: date Something is referring to the date module deprecation A deprecation was made in the PR labels Apr 17, 2023
@Shinigami92 Shinigami92 requested a review from a team as a code owner April 17, 2023 12:59
@Shinigami92 Shinigami92 self-assigned this Apr 17, 2023
@Shinigami92
Copy link
Member Author

@ST-DDT How can I fix the deprecation test without adding multiple signature overloads to the methods? These would be overkill in my opinion for just deprecating/renaming a property from the options.

@ST-DDT
Copy link
Member

ST-DDT commented Apr 17, 2023

@ST-DDT How can I fix the deprecation test without adding multiple signature overloads to the methods? These would be overkill in my opinion for just deprecating/renaming a property from the options.

That is sadly not possible. When typing a method or something, Ts always takes the last signature.
This also affected the type FakerOptions = ConstructorParameter[0]. As well as our doc generation.

@Shinigami92
Copy link
Member Author

This also affected the type FakerOptions = ConstructorParameter[0]. As well as our doc generation.

Should I add an ignore to the test or really quadruple the signature?

@ST-DDT
Copy link
Member

ST-DDT commented Apr 17, 2023

Should I add an ignore to the test or really quadruple the signature?

For consistency reasons, please add the mixed signature.

@Shinigami92
Copy link
Member Author

For consistency reasons, please add the mixed signature.

done

@codecov
Copy link

codecov bot commented Apr 17, 2023

Codecov Report

Merging #2068 (3b41abc) into next (db8792c) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2068      +/-   ##
==========================================
- Coverage   99.61%   99.61%   -0.01%     
==========================================
  Files        2535     2535              
  Lines      241892   242163     +271     
  Branches     1287     1291       +4     
==========================================
+ Hits       240970   241232     +262     
- Misses        895      904       +9     
  Partials       27       27              
Impacted Files Coverage Δ
src/modules/date/index.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@ST-DDT
Copy link
Member

ST-DDT commented Apr 17, 2023

The CI still fails.

@Shinigami92
Copy link
Member Author

Shinigami92 commented Apr 17, 2023

The CI still fails.

ts-check:tests is not in the preflight script 🤔

oh it is! but it does not exit !0 👀

src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
src/modules/date/index.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 requested a review from ST-DDT April 17, 2023 15:23
@ST-DDT ST-DDT requested review from a team April 17, 2023 15:24
@ST-DDT ST-DDT added p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug labels Apr 17, 2023
@Shinigami92 Shinigami92 merged commit fc13424 into next Apr 18, 2023
@Shinigami92 Shinigami92 deleted the rename-date-abbr-to-abbreviated branch April 18, 2023 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs deprecation A deprecation was made in the PR m: date Something is referring to the date module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants