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

feat(airline): add airline module #1699

Merged
merged 75 commits into from
Jan 30, 2023
Merged

feat(airline): add airline module #1699

merged 75 commits into from
Jan 30, 2023

Conversation

matthewpetro
Copy link
Contributor

Added a new module titled 'airline' for generating data pertaining to airlines.

@matthewpetro matthewpetro requested a review from a team December 30, 2022 17:48
@matthewpetro matthewpetro requested a review from a team as a code owner December 30, 2022 17:48
ST-DDT
ST-DDT reviewed Dec 30, 2022
View reviewed changes
src/locales/en/airline/airport.ts Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Dec 30, 2022 •
edited
Loading

Codecov Report

Merging #1699 (b2ebec8) into next (05d6eb4) will decrease coverage by 0.01%.
The diff coverage is 97.02%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             next    #1699      +/-   ##
==========================================
- Coverage   99.64%   99.63%   -0.01%     
==========================================
  Files        2340     2346       +6     
  Lines      234260   235000     +740     
  Branches     1116     1133      +17     
==========================================
+ Hits       233427   234145     +718     
- Misses        812      833      +21     
- Partials       21       22       +1
Impacted Files Coverage Δ
src/definitions/airline.ts 0.00% <0.00%> (ø)
src/definitions/definitions.ts 0.00% <0.00%> (ø)
src/definitions/index.ts 0.00% <0.00%> (ø)
src/faker.ts 95.46% <100.00%> (+0.02%) ⬆️
src/index.ts 100.00% <100.00%> (ø)
src/locales/en/airline/airline.ts 100.00% <100.00%> (ø)
src/locales/en/airline/airplane.ts 100.00% <100.00%> (ø)
src/locales/en/airline/airport.ts 100.00% <100.00%> (ø)
src/locales/en/airline/index.ts 100.00% <100.00%> (ø)
src/locales/en/index.ts 100.00% <100.00%> (ø)
... and 3 more

@ST-DDT ST-DDT added c: feature Request for new feature p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision labels Dec 30, 2022
import-brain
import-brain reviewed Dec 31, 2022
View reviewed changes
Copy link
Member

@import-brain import-brain left a comment

Choose a reason for hiding this comment

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

Please update the snapshot tests

Shinigami92
Shinigami92 reviewed Dec 31, 2022
View reviewed changes
Copy link
Member

@Shinigami92 Shinigami92 left a comment

Choose a reason for hiding this comment

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

Please rebase and rerun format and fix the lint errors

README.md Outdated Show resolved Hide resolved
ST-DDT
ST-DDT reviewed Dec 31, 2022
View reviewed changes
Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

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

Also please fix the lint error(s).

src/modules/airline/index.ts Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
src/modules/airline/index.ts Outdated Show resolved Hide resolved
import-brain
import-brain previously approved these changes Jan 29, 2023
View reviewed changes
@import-brain import-brain added the needs rebase There is a merge conflict label Jan 29, 2023
@ST-DDT ST-DDT removed the needs rebase There is a merge conflict label Jan 29, 2023
ST-DDT
ST-DDT requested changes Jan 29, 2023
View reviewed changes
src/locales/en/index.ts Outdated Show resolved Hide resolved
@Shinigami92
Copy link
Member

Should this be merged before we decide if we still want to have "complex" objects in definitions? Or should we wait a bit (could be around 3 weeks due to no meetings)

Here we have iataCodes which can be put into global definitions, but airport names are in english and could be translated to their individual locales 🤔
I feel not really happy with this design, but it's not an issue by this module itself but faker itself right now

@ST-DDT
Copy link
Member

ST-DDT commented Jan 30, 2023

Lets get this merged, and decide later.
We have all the data we need here and can strip unwanted/restructure them later easily.

@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Jan 30, 2023
@Shinigami92
Copy link
Member

@ST-DDT there is one unresolved discussion
please reevaluate it and decide if we can merge or need to do further actions on that

@ST-DDT ST-DDT enabled auto-merge (squash) January 30, 2023 21:07
@ST-DDT ST-DDT merged commit 579c9ad into faker-js:next Jan 30, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Jan 30, 2023

Thanks for your contribution.

@matthewpetro
Copy link
Contributor Author

You're welcome!

@matthewmayer matthewmayer mentioned this pull request Feb 9, 2023
Closed
matthewmayer pushed a commit to matthewmayer/faker that referenced this pull request Feb 18, 2023
@matthewpetro @matthewmayer
feat(airline): add airline module (faker-js#1699)
a200e97
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@import-brain import-brain import-brain approved these changes

@Shinigami92 Shinigami92 Shinigami92 approved these changes

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

@matthewmayer matthewmayer Awaiting requested review from matthewmayer

@xDivisionByZerox xDivisionByZerox Awaiting requested review from xDivisionByZerox

Assignees

@matthewpetro matthewpetro

Labels
c: feature Request for new feature p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
v8.x
Development

Successfully merging this pull request may close these issues.
5 participants
@matthewpetro @matthewmayer @Shinigami92 @ST-DDT @import-brain