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

refactor(finance): rename account to accountNumber #2054

Merged
merged 12 commits into from
Apr 18, 2023
Merged

refactor(finance): rename account to accountNumber #2054

merged 12 commits into from
Apr 18, 2023

Conversation

Shinigami92
Copy link
Member

references #1346 (comment)

@Shinigami92 Shinigami92 added c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: finance Something is referring to the finance module labels Apr 15, 2023
@Shinigami92 Shinigami92 requested a review from a team as a code owner April 15, 2023 17:16
@Shinigami92 Shinigami92 self-assigned this Apr 15, 2023
@codecov
Copy link

codecov bot commented Apr 15, 2023 •
edited
Loading

Codecov Report

Merging #2054 (92e5a06) into next (fc13424) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files 
@@           Coverage Diff           @@
##             next    #2054   +/-   ##
=======================================
  Coverage   99.61%   99.61%           
=======================================
  Files        2535     2535           
  Lines      242163   242190   +27     
  Branches     1291     1292    +1     
=======================================
+ Hits       241220   241255   +35     
+ Misses        916      908    -8     
  Partials       27       27
Impacted Files Coverage Δ
src/modules/finance/index.ts 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@xDivisionByZerox xDivisionByZerox mentioned this pull request Apr 15, 2023
Closed
@Shinigami92 Shinigami92 marked this pull request as draft April 15, 2023 17:28
xDivisionByZerox
xDivisionByZerox requested changes Apr 15, 2023
View reviewed changes
Copy link
Member

@xDivisionByZerox xDivisionByZerox left a comment

Choose a reason for hiding this comment

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

Please include this in the upgrading guide.

src/modules/finance/index.ts Outdated Show resolved Hide resolved
src/modules/finance/index.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 force-pushed the rename-account-to-account-number branch from 2e603cd to 7dd19be Compare April 15, 2023 17:34
@Shinigami92 Shinigami92 marked this pull request as ready for review April 15, 2023 17:35
@ST-DDT ST-DDT added the p: 1-normal Nothing urgent label Apr 15, 2023
@Shinigami92 Shinigami92 requested a review from a team April 16, 2023 10:32
ST-DDT
ST-DDT previously approved these changes Apr 16, 2023
View reviewed changes
@ST-DDT ST-DDT requested a review from a team April 16, 2023 10:38
@ST-DDT ST-DDT added the s: accepted Accepted feature / Confirmed bug label Apr 16, 2023
@ST-DDT ST-DDT requested a review from a team April 17, 2023 08:12
ejcheng
ejcheng requested changes Apr 18, 2023
View reviewed changes
test/finance.spec.ts Outdated Show resolved Hide resolved
test/finance.spec.ts Outdated Show resolved Hide resolved
test/finance.spec.ts Outdated Show resolved Hide resolved
@Shinigami92 Shinigami92 dismissed stale reviews from xDivisionByZerox and ST-DDT via a360d8e April 18, 2023 06:21
@Shinigami92 Shinigami92 merged commit 5c3b6e0 into next Apr 18, 2023
@Shinigami92 Shinigami92 deleted the rename-account-to-account-number branch April 18, 2023 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ejcheng ejcheng ejcheng approved these changes

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

@matthewmayer matthewmayer matthewmayer approved these changes

@xDivisionByZerox xDivisionByZerox Awaiting requested review from xDivisionByZerox

Assignees

@Shinigami92 Shinigami92

Labels
c: refactor PR that affects the runtime behavior, but doesn't add new features or fixes bugs m: finance Something is referring to the finance module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Faker Roadmap
Status: Done
Milestone
v8.0 - Module Re-Shuffling
Development

Successfully merging this pull request may close these issues.
5 participants
@Shinigami92 @matthewmayer @ST-DDT @ejcheng @xDivisionByZerox