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 Polling Backend and Sync service #3799

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

max-nextcloud
Copy link
Collaborator

📝 Summary

Refactor the Polling Backend and Synchronization service

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing ... let's see
  • Documentation is not required

@cypress
Copy link

cypress bot commented Feb 16, 2023

1 failed and 1 flaky tests on run #8658 ↗︎

1 134 0 0 Flakiness 1

Details:

Refactor Polling Backend and Sync service
Project: Text Commit: 742edb3212
Status: Failed Duration: 03:35 💡
Started: Feb 20, 2023 6:52 AM Ended: Feb 20, 2023 6:55 AM
Failed  cypress/e2e/conflict.spec.js • 1 failed test

View Output Video

Test
Open test.md in viewer > displays conflicts Screenshot
Flakiness  cypress/e2e/share.spec.js • 1 flaky test

View Output Video

Test
Open test.md in viewer > Share a file with download disabled shows an error Screenshot

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Signed-off-by: Max <max@nextcloud.com>
Both were kept up to date but never used as far as i can tell.

Signed-off-by: Max <max@nextcloud.com>
Save will only save the doc and doc state.
Sync will only fetch steps from the server.

Signed-off-by: Max <max@nextcloud.com>
@juliushaertl
Copy link
Member

@max-nextcloud Considering the amount of conflicts, is this something to rather do in smaller iterations? Can we extract some parts easily? Any plan to still continue or shall we close this for now?

@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Jan 5, 2024

is this something to rather do in smaller iterations?

Yes. It's basically 3 things:

  1. cleanup of unused instance variables in SyncService - merged in cleanup(SyncService): this.sessions and this.steps #5188
  2. separation of save and sync backend calls - already implemented by blizzz in main
  3. prep for a simpler polling timer. Still needs to be finished.

Can we extract some parts easily?

  1. was easy. 2. has some changes on top of what landed in master that I want to take another look at. 3. is not done yet but completely separate.

Any plan to still continue or shall we close this for now?

My plan is to get one of these 3 parts ready for merge every week. This week I did 1.

@max-nextcloud max-nextcloud removed this from the Nextcloud 29 milestone Mar 14, 2024
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.

None yet

3 participants