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

chore: enforce proper types in sync/index #1077

Merged
merged 10 commits into from
Jun 25, 2023

Conversation

TomAFrench
Copy link
Contributor

I've updated the return values in sync/index to sensible values based on how they're being called.

I've updated the type of msg.value to be string | number | null to match the values expected by serializeValue

@netlify
Copy link

netlify bot commented May 30, 2023

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit 01241f7
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/64972b2535fa6b000816619d
😎 Deploy Preview https://deploy-preview-1077--actualbudget.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@TomAFrench TomAFrench marked this pull request as ready for review May 30, 2023 14:44
@MatissJanis
Copy link
Member

👋 Looks like the e2e tests are failing. Smells like this is not just a type change, but some functionality has changed too. Would you mind taking a look?

@MatissJanis
Copy link
Member

Hey! Just checking in: how's this going? Did you see my last message?

I'd be very excited to get this merged as long as it doesn't break things and CICD passes.

* master: (54 commits)
  New linter rule import/no-unused-modules & fixing import on typescript (actualbudget#1173)
  React Router 6 fixes (actualbudget#1178)
  Remove remaining tutorial code (actualbudget#1174)
  react-router 6 upgrade (actualbudget#1066)
  Deleting all unused files, deleting unused functions from loot-core (actualbudget#1158)
  Log more details when migrations are out of sync (actualbudget#1161)
  Remove upgrade notifications code (actualbudget#1156)
  🔥  removing needs-triage github label (actualbudget#1157)
  Tidy up exports in loot-core (actualbudget#1147)
  🐛  remove 'we have been notified' copy (actualbudget#1155)
  Updates to the server button at the top right (actualbudget#1141)
  Expand / collapse all categories (actualbudget#1143)
  ✨ (nordigen) release the feature (actualbudget#1135)
  Improve error logging in the API (actualbudget#1121)
  ♻️ (crdt) moved re-used utils in actual-server to separate package (actualbudget#1150)
  Removing Tutorial code (actualbudget#1146)
  Removing unused functions (actualbudget#1145)
  Revert “Make number parsing agnostic to decimal and thousands separators” (actualbudget#1144)
  Strip a trailing slash off of server URLs (actualbudget#1140)
  Update CONTRIBUTING.md to point to the website (actualbudget#1138)
  ...
@TomAFrench
Copy link
Contributor Author

Hey @MatissJanis, sorry I only saw these messages recently (I'm travelling currently plus github notifications tend to be a bit of an unending stream). I've merged in the changes from master and the e2e tests seem to have fixed themselves. I'm not sure if they were just being flakey or some of the changes from master helped fix it.

Unfortunately, I've only got my laptop with me so I can't do a huge amount of extra investigation (If I run the debug builds on my laptop then it starts really chugging).

@MatissJanis
Copy link
Member

Thanks!

@MatissJanis MatissJanis merged commit fddcdec into actualbudget:master Jun 25, 2023
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved ✨ Merged Pull Request has been merged successfully labels Jun 25, 2023
@TomAFrench TomAFrench deleted the message-types branch June 25, 2023 18:17
TomAFrench added a commit to TomAFrench/actual that referenced this pull request Jun 26, 2023
* master:
  ⬆️  upgrade @reach/listbox and remove monkeypatch (actualbudget#1190)
  🔥  remove beta code and some unused scripts (actualbudget#1189)
  Goals: Use shared 'months' functions for time (actualbudget#1082)
  Fix completed feature request handling (actualbudget#1183)
  Fix “delete file” modal layout (actualbudget#1170)
  chore: enforce proper types in `sync/index` (actualbudget#1077)
  Fix transaction list page being blank on mobile (actualbudget#1171)
  Automatic category selection for new category transactions (actualbudget#1176)
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
I've updated the return values in `sync/index` to sensible values based
on how they're being called.

I've updated the type of `msg.value` to be `string | number | null` to
match the values expected by `serializeValue`
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.

3 participants