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

refactor: use workspaces to enable testing of bumped versions #646

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

morremeyer
Copy link
Contributor

@morremeyer morremeyer commented May 24, 2023

This PR makes use of yarn workspaces to enable local testing even when versions are bumped.

It also:

  • bumps various dependencies to up-to-date versions so that the build and type checking are successful.
  • removes node 16 from the testing workflows since current versions of Gatsby require at least node 18
  • adds tests for both the bank2ynab-converter and the web-app
  • adds a results-${package} job to every test workflow that checks the result of the matrix job. With that, the required checks can be updated to only require the following jobs: results-bank2ynab-converter, results-parsers, results-web-app instead of updating the required checks every time a new node version is added to the matrix job.

There's two things that need careful review:

  • I had to append /src to the imports of ynap-parsers and ynap-bank2ynab-converter. I have no idea if this is how it should be or if it will break when the packages are released .
  • I had to add an import for Buffer in the web app to fix a Buffer is undefined error. I tested the web-app by running yarn build and yarn start and then converting a file.

@leolabs This change will enable #619 to run successful tests and be merged.

@codecov
Copy link

codecov bot commented May 24, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 🎉

Comparison is base (89a4907) 91.09% compared to head (34195eb) 91.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #646      +/-   ##
==========================================
+ Coverage   91.09%   91.10%   +0.01%     
==========================================
  Files          25       25              
  Lines         865      866       +1     
  Branches      234      234              
==========================================
+ Hits          788      789       +1     
  Misses         70       70              
  Partials        7        7              
Impacted Files Coverage Δ
packages/ynap-parsers/src/bank2ynab/bank2ynab.ts 68.96% <ø> (ø)
...ackages/ynap-parsers/src/util/read-encoded-file.ts 94.44% <100.00%> (+0.32%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@morremeyer morremeyer marked this pull request as draft May 25, 2023 09:29
@morremeyer morremeyer force-pushed the refactor/move-to-workspaces branch from d58f15f to 7232371 Compare May 26, 2023 08:17
@morremeyer morremeyer marked this pull request as ready for review May 26, 2023 08:29
@morremeyer
Copy link
Contributor Author

@leolabs This is now ready for review, all notes above. If you're happy with it, can you also update the required checks for the PRs as described?

Thanks!

@morremeyer morremeyer force-pushed the refactor/move-to-workspaces branch 2 times, most recently from 6793a75 to 3da0349 Compare June 30, 2023 09:42
@morremeyer
Copy link
Contributor Author

Merged this in the fork with envelope-zero#1

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.

None yet

1 participant