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

[Fix] node/git isClean to match returned type #3022

Merged
merged 3 commits into from
Oct 30, 2023

Conversation

juanpprieto
Copy link
Contributor

@juanpprieto juanpprieto commented Oct 26, 2023

WHY are these changes introduced?

While using git isClean I noticed the return type Promise<boolean> did not match the actual return of the function.

simple-git's git.status().isClean is a function and not a property.

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@github-actions

This comment has been minimized.

@juanpprieto juanpprieto requested review from a team, Arkham and gonzaloriestra and removed request for a team October 26, 2023 17:05
Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Good catch, thanks! Fortunately, it wasn't being used in the repository... Could you fix the tests?

@github-actions
Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
73.07% (-0.01% 🔻)
6184/8463
🟡 Branches
69.93% (-0.01% 🔻)
3026/4327
🟡 Functions
71.45% (-0.03% 🔻)
1569/2196
🟡 Lines
74.3% (-0.01% 🔻)
5871/7902
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / ui.tsx
100%
90% (-1.67% 🔻)
100% 100%
🟢
... / urls.ts
85.45% (-0.13% 🔻)
81.82% (-1.3% 🔻)
81.25%
87.5% (-0.12% 🔻)

Test suite run success

1452 tests passing in 675 suites.

Report generated by 🧪jest coverage report action from 7dfb7c3

@juanpprieto
Copy link
Contributor Author

👋🏼 @gonzaloriestra I think these test updates are ok?

Copy link
Contributor

@gonzaloriestra gonzaloriestra left a comment

Choose a reason for hiding this comment

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

Thanks! 👌

@gonzaloriestra gonzaloriestra added this pull request to the merge queue Oct 30, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 30, 2023
@gonzaloriestra gonzaloriestra added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main with commit d31451e Oct 30, 2023
@gonzaloriestra gonzaloriestra deleted the juanpprieto/fix-node-git-is-clean branch October 30, 2023 10:49
@shopify-shipit shopify-shipit bot temporarily deployed to nightly October 30, 2023 21:30 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants