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.

Sign up for GitHub

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

Jump to bottom

fix: some return types #307

Merged
merged 3 commits into from
Jan 28, 2022
Merged

fix: some return types #307

merged 3 commits into from
Jan 28, 2022

Conversation

import-brain
Copy link
Member

Partially fixes #305

@import-brain import-brain requested a review from a team as a code owner January 26, 2022 03:23
@import-brain import-brain changed the title fix: some implicit return types fix: some return types Jan 26, 2022
@Shinigami92
Copy link
Member

Currently one of our problems is that arrayElement doesn't guarantee type-safetynes (regrading to compile-time).
Our definitions currently are just mostly arrays, but could be anything 🤷
I know that there are currently mostly string[] and we could just take over the types of @types/faker, but I would like to double-check that instead of assuming.

@Shinigami92 Shinigami92 added the c: chore PR that doesn't affect the runtime behavior label Jan 26, 2022
ST-DDT
ST-DDT previously approved these changes Jan 26, 2022
View reviewed changes
@ST-DDT
Copy link
Member

ST-DDT commented Jan 26, 2022

IMO we should set these to the expected return types. Double checking with the existing ones would still be a good idea.

doughlass
doughlass previously approved these changes Jan 26, 2022
View reviewed changes
@import-brain
Copy link
Member Author

import-brain commented Jan 26, 2022 •
edited
Loading

IMO we should set these to the expected return types. Double checking with the existing ones would still be a good idea.

All of the ones in this PR were checked against @types/faker latest version (before corruption), they all seem reasonable and match up.

@ST-DDT ST-DDT requested a review from a team January 26, 2022 12:10
@import-brain import-brain dismissed stale reviews from doughlass and ST-DDT via 9577d68 January 26, 2022 18:18
@ST-DDT
Copy link
Member

ST-DDT commented Jan 26, 2022

A probably wrong commit ended up here.

ST-DDT
ST-DDT requested changes Jan 26, 2022
View reviewed changes
.github/ISSUE_TEMPLATE/config.yml Outdated Show resolved Hide resolved
@demipel8 demipel8 mentioned this pull request Jan 27, 2022
Merged
@demipel8
Copy link
Contributor

Wrong mention, sorry.

@Shinigami92 Shinigami92 merged commit 4ca61ca into faker-js:main Jan 28, 2022
bmenant pushed a commit to bmenant/faker that referenced this pull request Mar 11, 2022
@import-brain @bmenant
fix: some return types (faker-js#307)
ae16f89
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
@import-brain @demipel8
fix: some return types (faker-js#307)
b1b1438
demipel8 pushed a commit to demipel8/faker that referenced this pull request Mar 11, 2022
@import-brain @demipel8
fix: some return types (faker-js#307)
963a27d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shinigami92 Shinigami92 Shinigami92 approved these changes

@ST-DDT ST-DDT ST-DDT approved these changes

@doughlass doughlass doughlass left review comments

@damienwebdev damienwebdev Awaiting requested review from damienwebdev

Assignees

@import-brain import-brain

Labels
c: chore PR that doesn't affect the runtime behavior
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Vet v6.0.0-alpha.5 types emitted types against v5.5.3
5 participants
@import-brain @Shinigami92 @ST-DDT @demipel8 @doughlass