Skip to content

chore: to replace fs.access with fs.exist#1890

Merged
tdeekens merged 2 commits intomasterfrom
to-replace-fs.exists-with-fs.access
Mar 18, 2025
Merged

chore: to replace fs.access with fs.exist#1890
tdeekens merged 2 commits intomasterfrom
to-replace-fs.exists-with-fs.access

Conversation

@Rombelirk
Copy link
Contributor

@Rombelirk Rombelirk self-assigned this Apr 30, 2024
@changeset-bot
Copy link

changeset-bot bot commented Apr 30, 2024

🦋 Changeset detected

Latest commit: 61ead31

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
@commercetools/csv-parser-discount-code Patch
@commercetools/discount-code-generator Patch
@commercetools/discount-code-exporter Patch
@commercetools/inventories-exporter Patch
@commercetools/product-json-to-xlsx Patch
@commercetools/product-json-to-csv Patch
@commercetools/csv-parser-state Patch
@commercetools/price-exporter Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codecov
Copy link

codecov bot commented May 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.76%. Comparing base (2a417d2) to head (61ead31).
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1890   +/-   ##
=======================================
  Coverage   97.76%   97.76%           
=======================================
  Files         147      147           
  Lines        4786     4786           
  Branches     1283     1283           
=======================================
  Hits         4679     4679           
  Misses        104      104           
  Partials        3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.


import CsvParserDiscountCode from './main'

const doesFileExist = (filePath) => {
Copy link
Member

Choose a reason for hiding this comment

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

Just a little observation, it will be nice if we can add a simple unit test to test this function.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but then we have a cross package dependency. We don't seen to have a shared package so duplication should be fine.

@ajimae
Copy link
Member

ajimae commented May 30, 2024

Also, to make the testing easier, I will suggest you move doesFileExist as an exported function into a util or helper file and import them everywhere else where it is used.


import CsvParserDiscountCode from './main'

const doesFileExist = (filePath) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes but then we have a cross package dependency. We don't seen to have a shared package so duplication should be fine.

@tdeekens tdeekens force-pushed the to-replace-fs.exists-with-fs.access branch from f6c38b6 to 61ead31 Compare March 18, 2025 11:55
@tdeekens tdeekens merged commit 6799954 into master Mar 18, 2025
11 checks passed
@tdeekens tdeekens deleted the to-replace-fs.exists-with-fs.access branch March 18, 2025 12:21
@ct-changesets ct-changesets bot mentioned this pull request Mar 18, 2025
barbara79 pushed a commit that referenced this pull request Mar 19, 2025
* chore: to replace fs.access with fs.exist

* chore: changeset

---------

Co-authored-by: tdeekens <tobias.deekens@commercetools.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants