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

TypeScript: convert storyProvider #12652

Merged
merged 113 commits into from
Dec 2, 2022
Merged

TypeScript: convert storyProvider #12652

merged 113 commits into from
Dec 2, 2022

Conversation

miina
Copy link
Contributor

@miina miina commented Nov 8, 2022 •
edited
Loading

Context

Summary

Relevant Technical Choices

To-do

User-facing changes

Testing Instructions

  • This is a non-user-facing change and requires no QA

This PR can be tested by following these steps:

Reviews

Does this PR have a security-related impact?

Does this PR change what data or activity we track or use?

Does this PR have a legal-related impact?

Checklist

  • This PR addresses an existing issue and I have linked this PR to it in ZenHub
  • I have tested this code to the best of my abilities
  • I have verified accessibility to the best of my abilities (docs)
  • I have verified i18n and l10n (translation, right-to-left layout) to the best of my abilities
  • This code is covered by automated tests (unit, integration, and/or e2e) to verify it works as intended (docs)
  • I have added documentation where necessary
  • I have added a matching Type: XYZ label to the PR

Fixes #12411
Fixes #12657

@miina miina added Type: Infrastructure Changes impacting testing infrastructure or build tooling Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Pod: Prometheus labels Nov 8, 2022
@miina miina self-assigned this Nov 8, 2022
@miina miina changed the title TypeScript: conver storyProvider TypeScript: convert storyProvider Nov 8, 2022
barklund
barklund reviewed Dec 1, 2022
View reviewed changes
Copy link
Contributor

@barklund barklund left a comment

Choose a reason for hiding this comment

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

I made it through! Looks great, surprisingly few edits to the actual reducers and most of them are idiot-edits (checking for null for things we know can't be null just to make the compiler happy).

packages/story-editor/src/app/story/actions/useLocalAutoSave.ts Show resolved Hide resolved
packages/story-editor/src/app/story/effects/useHistoryEntry.ts Outdated Show resolved Hide resolved
packages/story-editor/src/app/story/effects/useLoadStory.ts Outdated Show resolved Hide resolved
packages/story-editor/src/utils/getUniquePresets.ts Outdated Show resolved Hide resolved
packages/story-editor/src/app/story/useStoryReducer/reducers/addAnimations.ts Outdated Show resolved Hide resolved
packages/story-editor/src/app/story/effects/useHistoryEntry.ts Show resolved Hide resolved
packages/story-editor/src/app/story/utils/pageContainsBlobUrl.ts Outdated Show resolved Hide resolved
packages/story-editor/src/typings/global.d.ts Show resolved Hide resolved
packages/story-editor/src/utils/useRefreshPostEditURL.ts Show resolved Hide resolved
packages/story-editor/tsconfig.json Outdated Show resolved Hide resolved
@miina
Copy link
Contributor Author

miina commented Dec 1, 2022

Uh, still 2 Karma tests failing. Looking into it, otherwise all passing finally 🎉

barklund
barklund approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@barklund barklund left a comment

Choose a reason for hiding this comment

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

🥋

@barklund barklund mentioned this pull request Dec 2, 2022
Merged
11 tasks
@spacedmonkey
Copy link
Contributor

@miina This PR needs to rebased after #12625 has been merged.

@miina miina merged commit 550ff16 into main Dec 2, 2022
@miina miina deleted the code/12411-story-provider-ts branch December 2, 2022 15:41
@miina
Copy link
Contributor Author

miina commented Dec 2, 2022

The failing test was the same flaky one as on main, so merged with that.

@timarney timarney mentioned this pull request Dec 2, 2022
Merged
8 tasks
@spacedmonkey spacedmonkey mentioned this pull request Dec 12, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@barklund barklund barklund approved these changes

@merapi merapi Awaiting requested review from merapi

@swissspidy swissspidy Awaiting requested review from swissspidy

@spacedmonkey spacedmonkey Awaiting requested review from spacedmonkey

Assignees

@miina miina

Labels
Type: Code Quality Things that need a refactor, rewrite or just some good old developer ❤️ Type: Infrastructure Changes impacting testing infrastructure or build tooling
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TypeScript: Convert snackbar provider TypeScript: Convert story provider
5 participants
@miina @googleforcreators-bot @spacedmonkey @barklund @swissspidy