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

Working Location-Watcher system for iOS app #2073

Merged
merged 44 commits into from
Feb 29, 2024

Conversation

Rocky43007
Copy link
Member

@Rocky43007 Rocky43007 commented Feb 9, 2024

The iOS app has been in a state of completion for a while now with a working location watcher system. So, I'm splitting PR #1812 into 2 PRs, because Android is being a pain to develop for. This means that the iOS app exists in a state of actual functionality for everyday use.

Blocked on PR #2071 because there's been a bunch of documentation changes, and that has the latest docs changes.
Edit: PR #2071 was Merged

TODO List for this PR:

  • Test for notify-rs on web and desktop applications
  • Write tests for the iOS Event Handler in core/src/locations/watcher/mod.rs
  • Clean up

Summary by CodeRabbit

  • New Features
    • Introduced a new LocationOnboarding screen for enhanced file location management in the mobile app.
    • Added functionality for Android and iOS file system watchers in the core system.
  • Enhancements
    • Improved navigation by adding the LocationOnboarding screen to the navigation structure.
    • Adjusted app title rendering logic for consistent capitalization across platforms.
  • Refactor
    • Cleaned up code and improved logging in various core components, focusing on location management and event handling.
    • Reformatted tab configurations in the mobile app without altering functionality.
  • Chores
    • Added a script for cleaning target folders specific to Android or iOS platforms.
  • Style
    • Minor style adjustments in Location screen rendering.

We can now add Downloads (A location locked by MANGE_EXTERNAL_STORAGE) on Android.
Adding location now navigates to the added location explorer.
Found a way simpler solution, that doesn't require query calls.
Remove unused import calls.
Well, Location watcher doesn't want to run. But I wanted to push the `xcrun` command to the README.

Also, locations now actually show files, but break on displaying recursive folders (folder content in a folder).
locations.fullRescan works now on mobile with 0 issues. It can recognize orphan paths and when new files are added.

But, Location-Watcher for some reason doesn't want to run automatically when the file system change occurs.
Location Watcher now works for iOS when files are created. However, it can't understand the file delete event yet.
The watcher system is now fully functional on iOS, with a few bugs to patch regarding the delete function freezing the system up.
Finally figured out how to see the logs from Android.
commit a7bc3b9
Merge: 201913a 2899aa3
Author: Arnab Chakraborty <11457760+Rocky43007@users.noreply.github.com>
Date:   Tue Feb 6 16:18:42 2024 -0500

    Merge branch 'main' of https://github.com/Rocky43007/spacedrive

commit 2899aa3
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 6 19:30:53 2024 +0300

    [ENG-1594] Change online to connected (spacedriveapp#2060)

    : This is a combination of 3 commits.

    Change online to connected

    remove offline

    json

commit 48634c2
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 6 17:49:16 2024 +0300

    [MOB-54] UI Fixes (spacedriveapp#2059)

    * UI fixes - rive animation - SD version in settings - and more

    * twStyle

commit 4e70246
Author: Jesse Rodrigo <39565615+JSSRDRG@users.noreply.github.com>
Date:   Tue Feb 6 11:41:05 2024 +0100

    Dutch locale (spacedriveapp#2054)

    * nl locale

    * add nl entry

    * improve some wording

commit bb0d0af
Author: Brendan Allan <brendonovich@outlook.com>
Date:   Tue Feb 6 17:13:47 2024 +0800

    [ENG-1548] use in-memory instances when sending messages to cloud (spacedriveapp#2057)

    * use in-memory instances when sending messages to cloud

    * comments

commit 2d0c340
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 6 06:03:33 2024 +0300

    [MOB-47] Location screen and header updates (spacedriveapp#2056)

    * Location screen and header updates

    * use tw sizing

    * remove un-necessary prop

    * Nit: change name

commit 58f9305
Author: Arnab Chakraborty <11457760+Rocky43007@users.noreply.github.com>
Date:   Mon Feb 5 07:06:47 2024 -0500

    Update to Expo 50 and Fix to Rive Crashing (spacedriveapp#2049)

    * Update Mobile App to Expo SDK 50
    + Fix to Rive Crashing

    * Added `metro-react-native-babel-transformer` to fix CI

commit 2ff1ffc
Author: Utku <74243531+utkubakir@users.noreply.github.com>
Date:   Sun Feb 4 23:52:26 2024 +0300

    Fix Chinese language (spacedriveapp#2050)

    * fix chinese

    * remove console.log
commit bda9a1b
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Wed Feb 7 20:25:04 2024 +0300

    [MOB-55] Video animation for onboarding on mobile and desktop (spacedriveapp#2065)

    * video animation for onboarding on mobile and desktop

    run assets gen

    cleanup

    declare mp4 type

    * update metro config to transform video files from sd assets

    * test ci without native video exclude

    * casing?

    * remove to add back again due to github

    * add videos back

    * versions

    * no need to transform

    ---------

    Co-authored-by: Utku Bakir <74243531+utkubakir@users.noreply.github.com>

commit da2841b
Author: Utku <74243531+utkubakir@users.noreply.github.com>
Date:   Wed Feb 7 16:47:55 2024 +0300

    More translations (spacedriveapp#2051)

    * translations

    * more translation keys

    * all the translations
Copy link

vercel bot commented Feb 9, 2024

@Rocky43007 is attempting to deploy a commit to the Spacedrive Team on Vercel.

A member of the Team first needs to authorize it.

PR 1812 was closed, therefore changing the message to mention the branch instead.
commit dba85eb
Author: Ericson "Fogo" Soares <ericson.ds999@gmail.com>
Date:   Mon Feb 26 16:45:58 2024 -0300

    [ENG-1513] Better integration between Jobs and processing Actors (spacedriveapp#1974)

    * First draft on new task system

    * Removing save to disk from task system

    * Bunch of concurrency issues

    * Solving Future impl issue when pausing tasks

    * Fix cancel and abort

    * Bunch of fixes on pause, suspend, resume, cancel and abort
    Also better error handling on task completion for the user

    * New capabilities to return an output on a task

    * Introducing a simple way to linear backoff on failed steal

    * Sample actor where tasks can dispatch more tasks

    * Rustfmt

    * Steal test to make sure

    * Stale deps cleanup

    * Removing unused utils

    * Initial lib docs

    * Docs ok

    * Memory cleanup on idle

    ---------

    Co-authored-by: Vítor Vasconcellos <vasconcellos.dev@gmail.com>

commit 53713a9
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Mon Feb 26 18:53:37 2024 +0300

    [ENG-1625] Spacedrop UI correct hover condition & spacing (spacedriveapp#2127)

    * improve spacedrop ui with correct hover & spacing

    * remove

commit 6f27504
Author: Matteo Galiazzo <50683509+gekoxyz@users.noreply.github.com>
Date:   Mon Feb 26 16:20:41 2024 +0100

    added it locale, added it entry (spacedriveapp#2066)

    * added it locale, added it entry

    * Apply suggestions from code review

    Co-authored-by: Matteo Martellini <matteo@mercxry.me>

    * add missing keys and a readme about the script

    ---------

    Co-authored-by: Utku <74243531+utkubakir@users.noreply.github.com>
    Co-authored-by: Matteo Martellini <matteo@mercxry.me>

commit 2832803
Author: jake <77554505+brxken128@users.noreply.github.com>
Date:   Mon Feb 26 15:17:28 2024 +0000

    Clean-up MacOS window closing behaviour code (spacedriveapp#2124)

    * fix: delete dead/unused file

    * refactor: add the window event handler with the rest of them

    * refactor: formatting

commit aa0b4ab
Author: Oscar Beaumont <oscar@otbeaumont.me>
Date:   Mon Feb 26 15:23:48 2024 +0800

    rspc over P2P (spacedriveapp#2112)

    * wip: rspc over p2p

    * wip

    * rspc over P2P

    * Cleanup + error handling

    * slight cleanup

    * Using Hyper for HTTP streaming + websockets

commit f7a7b00
Author: Utku <74243531+utkubakir@users.noreply.github.com>
Date:   Fri Feb 23 11:26:58 2024 -0500

    Fix android thumbs (spacedriveapp#2121)

    * replace react-native-fs with an active fork

    * time sink

    * fix

commit 6358c57
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Fri Feb 23 00:28:35 2024 +0300

    Mob: cleanup warning (spacedriveapp#2122)

    Update Categories.tsx

commit a4b7296
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Thu Feb 22 16:08:41 2024 +0300

    MOB: Settings paddings (spacedriveapp#2120)

    padding tweaks

commit d007b55
Author: nikec <43032218+niikeec@users.noreply.github.com>
Date:   Thu Feb 22 13:53:02 2024 +0100

    [ENG-1619] Add spacedrop to the context menu (spacedriveapp#2119)

    add spacedrop to context menu

commit dd0acad
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Thu Feb 22 15:47:08 2024 +0300

    [ENG-1618] Spacedrop UI (spacedriveapp#2118)

    * spacedrop ui update

    * i18n and description

    * glitch: remove !

    * already in progress i18n

    * more i18n

commit a17fb91
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Thu Feb 22 15:20:31 2024 +0300

    [MOB-62] Spacing & padding tweaks (spacedriveapp#2117)

    Spacing & padding tweaks

commit 6a32752
Author: Vítor Vasconcellos <vasconcellos.dev@gmail.com>
Date:   Thu Feb 22 03:15:36 2024 -0300

    Fix core test and CI breaking (spacedriveapp#2116)

    Fix core test passing inverted arguments to sync_db_entry macro

commit da4f038
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Wed Feb 21 17:19:40 2024 +0300

    Mob: better visually width fitting for categories (spacedriveapp#2114)

    * Visually width fitting for categories

    * remove padding

commit 3bdcc05
Author: Vítor Vasconcellos <vasconcellos.dev@gmail.com>
Date:   Wed Feb 21 11:18:15 2024 -0300

    Fix mobile CI + Some small CI improvements (spacedriveapp#2113)

    Fix mobile CI
     - Use rust envvars in all workflows
     - Use rust envvars and mold when build sd-server docker

commit b638fc2
Author: Utku <74243531+utkubakir@users.noreply.github.com>
Date:   Wed Feb 21 08:26:05 2024 -0500

    [MOB-37, MOB-38, MOB-39] Preview for PDF, Text and Media files (spacedriveapp#2098)

    * version & microphonePermission text & eslint

    * remove polyfill as hermes supports intl now

    * why do we have solid on mobile?

    * cleanup

    * add solid back =_=

    * pnpm lock

    * we hate relative paths here

    * android config

    * open file logic

    * visual tweaks

    ---------

    Co-authored-by: ameer2468 <33054370+ameer2468@users.noreply.github.com>

commit c533d12
Author: Brendan Allan <brendonovich@outlook.com>
Date:   Wed Feb 21 22:42:10 2024 +1100

    media data sync (spacedriveapp#2102)

    * basic sync operation backfill

    * media data sync

    * sync entry helpers

    * fix sync generator

    * nicer

    * re-add key_id

commit 393a907
Author: Vítor Vasconcellos <vasconcellos.dev@gmail.com>
Date:   Wed Feb 21 06:27:40 2024 -0300

    Update github actions due to nodejs 16 deprecation (spacedriveapp#2107)

    * Update github actions due to nodejs 16 deprecation
     - Replace archived actions-rs/clippy-check with maintained fork actions-rs-plus/clippy-check
     - Replace redhat-actions/push-to-registry with updated fork Eusebiotrigo/push-to-registry
     - Point redhat-actions/buildah-build and softprops/action-gh-release to current master to fix nodejs deprecation

    * Build the correct ios core rust arch for CI runs

    * Build ios app for the same arch as the host in Mobile CI

    * Some changes to try and make cache-factory faster and avoid failing so much

    * Add trigger to run cache-factory on pull requests when there are changes to itself

    * Attempt to fix sed usage on macOS

    * Don't treat warning as errors

    * Fix windows

    * Fix windows 2

    * Use target ad cache key for rust to differentiate between macOS x86_64 and arm64

    * Use faster/better linkers to compile for macOS, Linux and Windows

    * Fix missing shell in action

    * Fix typo

    * Fix missing shell in action 2

    * Fix mold download
     - Replace bsdtar with plain tar

    * Fix permission denied when extracting mold

    * Remove zld

    * Don't restore cache for rustfmt
     - Remove target symlink to C:/ in an attempt to speed-up windows CI

    * Fix typo

    * Restore target symlink on windows
     - Removing it didn't make CI faster

    * Run Mobile on macos-14

commit 519b1b6
Author: Oscar Beaumont <oscar@otbeaumont.me>
Date:   Wed Feb 21 16:13:40 2024 +0800

    Fix P2P not working for libraries (spacedriveapp#2031)

    * P2P Debug route

    * Remove legacy peer to peer pairing process

    * Fix error typo

    * Sync instances with cloud

    * Upgrade deps + extended instance data

    * Create instance with extended metadata

    * Auto sync instances

    * Actually `.await`

    * bruh

    * sync library info

    * this isn't gonna work

    * only sleep cloud receiver when no more messages (spacedriveapp#1985)

    * [ENG-1567] Fix renaming (spacedriveapp#1986)

    fix rename

    * only sleep cloud receiver when no more messages

    * use in memory instances during cloud receive (spacedriveapp#1995)

    * use in memory instances during cloud receive

    * is_empty

    ---------

    Co-authored-by: nikec <43032218+niikeec@users.noreply.github.com>

    * fix type error

    * wip

    * make mdns mdns better

    * rebuild state

    * Add hooks + listeners + discovered state

    * Split into crates

    * wip fixing core + wip merging Spacetime into `sd-p2p2`

    * `SmartLockGuard` + `Listener`

    * Make `sd-core` compile

    * Reenable all operation receivers

    * Fix all broken code within `sd-core`

    * minor fixes found in review

    * Bring in `libp2p` + restructure `sd-p2p` for the gazillion-th time

    * whoops

    * Compile no matter the (runtime) cost

    * fixing merge issues

    * wip

    * a

    * b

    * C

    * Handle port betterer

    * c

    * Migrate node config

    * a

    * no crash on startup

    * wip

    * a

    * jdfhskjfsg

    * a

    * fix discovery

    * a bunch of fixes

    * getting Spacedrop working

    * I don't get why it no worky

    * debug example

    * a

    * wip

    * wip

    * removing logging from stream impl

    * wip: shit is fucked

    * Redo quic integration  + Spacedrop working

    * Fix shutdown - deadlocks + shutdown peers

    * Add Prisma migrations

    * Fix shutdown

    * a

    * fix

    * cleanup

    * The lord clippy hath spoken

    * disable P2P settings for now

    ---------

    Co-authored-by: Brendan Allan <brendonovich@outlook.com>
    Co-authored-by: nikec <43032218+niikeec@users.noreply.github.com>

commit af8dbf7
Author: Julian Braha <julianbraha@gmail.com>
Date:   Tue Feb 20 20:06:05 2024 +0000

    Improve error handling by using decode::Error instead of io::Result (spacedriveapp#2078)

    Use decode::Error instead of io::Result

commit 84dadff
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 20 22:59:11 2024 +0300

    [MOB-59] Empty UI for locations and tags screen (spacedriveapp#2110)

    Empty UI for locations and tags screen

commit 9fc38d8
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 20 22:47:14 2024 +0300

    [MOB-60] locations & tags query invalidation on updates (spacedriveapp#2111)

    Trigger UI updates on location adding/delete and tags

    lint

    name

commit abd5ecb
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Tue Feb 20 13:05:53 2024 +0300

    mousedown fix (spacedriveapp#2108)

    quick mistake fix

commit 9bc1a47
Author: Brendan Allan <brendonovich@outlook.com>
Date:   Tue Feb 20 20:22:34 2024 +1100

    Basic sync operation backfill (spacedriveapp#2101)

    * basic sync operation backfill

    * no changes

commit 2a28347
Author: Arnab Chakraborty <11457760+Rocky43007@users.noreply.github.com>
Date:   Tue Feb 20 01:33:52 2024 -0500

    New Android Build Script (spacedriveapp#2096)

    * New Android Build Script

    * Clean up + Works for CI now

    * Simplify android build.sh
     - Fix /var/home/vitor fallback for Linux systems
     - Run a single cargo ndk for all targets (not parallel build, but a bit faster)
     - Fix android target s/x86/x86_64/
     - Format setup.sh
     - Minor improvements to rust mobile targets installation step in setup.sh

    * Add notice to CONTRIBUTING that only Java <= 17 is supported for building android
     - Make prettier ignore some mobile build artifacts

    * When in CI, Fix build android core for host architecture

    ---------

    Co-authored-by: Vítor Vasconcellos <vasconcellos.dev@gmail.com>

commit 19b2243
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Mon Feb 19 23:45:29 2024 +0300

    [ENG-1615] bg intro video fixed (spacedriveapp#2104)

    * video intro bg

    * test hsl

    * test video bg

    * run tests

    * comment

    * mob intro

    * git glitch

    * git

    * webm type

commit 4336060
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Mon Feb 19 19:12:11 2024 +0300

    [MOB-58] Settings routes new design & more (spacedriveapp#2103)

    * wip: redesigning settings pages

    * Edit location redesign & more

    * right actions

    * cleanup

commit e845082
Author: ameer2468 <33054370+ameer2468@users.noreply.github.com>
Date:   Mon Feb 19 18:23:20 2024 +0300

    [ENG-1612] Fix mouse nav forwards and backwards (spacedriveapp#2105)

    Fix mouse nav forwards and backwards
@Rocky43007 Rocky43007 marked this pull request as ready for review February 27, 2024 04:57
@Rocky43007 Rocky43007 requested review from a team as code owners February 27, 2024 04:57
@Rocky43007
Copy link
Member Author

Just tested the desktop build, works perfectly fine.

Copy link

coderabbitai bot commented Feb 28, 2024

Walkthrough

The updates encompass a wide range of changes, from configuration adjustments to scripting enhancements, mobile app development refinements, and core functionality improvements. These include updates to Rust analyzer settings in VS Code, the addition of cleaning scripts for mobile targets, navigation modifications in a mobile app, and significant enhancements in location management and file system watching for Android and iOS platforms. These changes collectively aim to boost development efficiency, streamline app navigation, and enhance file management capabilities, demonstrating a holistic effort to refine and enrich the application's functionality and developer experience.

Changes

Files Summary
.vscode/settings.json Updates related to rust-analyzer settings such as linkedProjects, cargo.extraEnv, check.targets, and showUnlinkedFileNotification.
apps/mobile/scripts/cleanTarget.sh Introduces a script for cleaning target folders based on specified platforms (Android or iOS).
apps/mobile/src/navigation/TabNavigator.tsx
apps/mobile/src/navigation/index.tsx
apps/mobile/src/screens/Location.tsx
apps/mobile/src/screens/settings/info/About.tsx
Navigation structure adjustments and minor UI refinements.
apps/mobile/src/screens/LocationOnboarding.tsx New file for managing file locations, enabling directory selection and existence validation.
core/src/api/locations.rs
core/src/lib.rs
core/src/location/manager/...
core/src/location/manager/watcher/...
Enhancements in location management, file system monitoring, and debugging across Android and iOS platforms.

🐰✨

In the realm of code, where changes abound,
A rabbit hopped in, improvements found.
With tweaks and tunes in every line,
Enhancing flow, making it shine.
From Android's depth to iOS's core,
This rabbit's work, forevermore,
Will guide the bytes, in harmony bind,
A digital world, refined, designed.
🌟🐾

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit-tests for this file.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository from git and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 16

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between dba85eb and 1c4fbff.
Files ignored due to path filters (4)
  • Cargo.lock is excluded by: !**/*.lock
  • apps/desktop/src-tauri/Cargo.toml is excluded by: !**/*.toml
  • apps/server/Cargo.toml is excluded by: !**/*.toml
  • core/Cargo.toml is excluded by: !**/*.toml
Files selected for processing (15)
  • .vscode/settings.json (1 hunks)
  • apps/mobile/scripts/cleanTarget.sh (1 hunks)
  • apps/mobile/src/navigation/TabNavigator.tsx (1 hunks)
  • apps/mobile/src/navigation/index.tsx (3 hunks)
  • apps/mobile/src/screens/Location.tsx (2 hunks)
  • apps/mobile/src/screens/LocationOnboarding.tsx (1 hunks)
  • apps/mobile/src/screens/settings/info/About.tsx (1 hunks)
  • core/src/api/locations.rs (2 hunks)
  • core/src/lib.rs (1 hunks)
  • core/src/location/manager/mod.rs (17 hunks)
  • core/src/location/manager/watcher/android.rs (1 hunks)
  • core/src/location/manager/watcher/ios.rs (1 hunks)
  • core/src/location/manager/watcher/mod.rs (14 hunks)
  • core/src/location/mod.rs (2 hunks)
  • core/src/object/media/mod.rs (1 hunks)
Files skipped from review due to trivial changes (4)
  • apps/mobile/src/navigation/TabNavigator.tsx
  • apps/mobile/src/screens/Location.tsx
  • apps/mobile/src/screens/settings/info/About.tsx
  • core/src/lib.rs
Additional comments: 19
apps/mobile/src/navigation/index.tsx (1)
  • 27-27: The addition of the LocationOnboarding screen to the RootStackParamList type is correctly implemented. This change will allow navigation to the LocationOnboarding screen within the app.
core/src/object/media/mod.rs (1)
  • 28-33: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The removal of the conditional compilation attribute from media_data_image_to_query_params function makes it available in all builds. Ensure that this change aligns with the project's requirements and that the function's availability does not introduce any unintended side effects.

.vscode/settings.json (1)
  • 88-91: The additions to .vscode/settings.json related to rust-analyzer settings are correctly implemented. These settings will help refine the development environment for Rust. Ensure that these settings align with the project's development practices and requirements.
core/src/location/manager/watcher/android.rs (1)
  • 155-183: The tick function correctly implements periodic checks and evictions based on elapsed time. However, ensure that the intervals (HUNDRED_MILLIS, ONE_SECOND) used for these checks are appropriately chosen based on the expected frequency of file system changes and the performance characteristics of the target devices.
core/src/api/locations.rs (1)
  • 29-29: The addition of trace logging is a good practice for detailed debugging and monitoring of the application's behavior. Ensure that the logging level is appropriately used to avoid excessive log noise in production environments.
core/src/location/manager/mod.rs (7)
  • 10-10: The addition of tracing_subscriber::field::debug import is noted. Ensure that this import is utilized effectively within the file, particularly in conjunction with the debug logging calls that have been added.
  • 24-24: The expanded import from tracing::{debug, error, info} is appropriate given the added logging statements throughout the file. This change enhances the readability and maintainability of the logging code.
  • 201-201: Adding a debug log at the start of the location manager actor is a good practice. It provides clear visibility into the lifecycle of the actor, which is beneficial for debugging and monitoring.
  • 230-230: The debug log added before sending a location management message is useful for tracking the actions being requested of the location manager. This addition aids in debugging and understanding the flow of location management actions.
  • 256-256: Similarly, the debug log before sending a watcher management message improves the traceability of watcher actions. This is particularly useful for diagnosing issues related to the starting, stopping, or reinitializing of watchers.
  • 403-403: Logging the online status of a location when it is being watched is a good practice. It provides immediate feedback on the operational status of the location, which is crucial for both monitoring and troubleshooting.
  • 553-553: The debug log indicating the stopping of the location manager is essential for understanding the lifecycle of the manager. It clearly marks the termination point of the manager's operation, which is valuable for debugging purposes.
core/src/location/manager/watcher/mod.rs (5)
  • 27-28: The addition of platform-specific modules for Android (android.rs) and iOS (ios.rs) is a significant enhancement for supporting these platforms. It's crucial to ensure that these modules are thoroughly tested, especially given the unique filesystem behaviors and event handling mechanisms on mobile platforms.

Ensure that comprehensive tests are in place for these new modules to verify their functionality across different scenarios and edge cases.

  • 158-158: Adding debug logging within the handle_watch_events function improves the visibility of the event handling process, which is beneficial for troubleshooting and understanding the flow of events. However, it's important to ensure that logging does not introduce performance overhead or leak sensitive information.
  • 246-246: The debug log statement at the beginning of the watch method provides clear feedback about the action being taken. This is a good practice for operations that might have a significant impact on the application's behavior or performance.
  • 691-697: The test case for the delete_file_event on iOS uses EventKind::Modify(ModifyKind::Metadata(MetadataKind::Any)) to expect a metadata modification event instead of a deletion event. This might be indicative of how iOS handles file deletions, but it's important to clarify this behavior in the test case or documentation to avoid confusion.

Consider adding a comment or documentation to explain why a metadata modification event is expected for file deletions on iOS, as this might not be intuitive for someone unfamiliar with iOS filesystem events.

  • 746-752: Similar to the previous comment, the expectation for a metadata modification event for directory deletions on iOS should be clarified. If this behavior is specific to iOS, documenting it will help maintainers and contributors understand the rationale behind this test expectation.

As with the file deletion event, ensure that there's an explanation or documentation for expecting a metadata modification event for directory deletions on iOS.

core/src/location/mod.rs (2)
  • 21-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

The removal of conditional compilation for the "location-watcher" feature suggests a shift towards making this functionality a core part of the application, regardless of the build configuration. This change could have implications for the application's behavior and performance across different platforms. It's crucial to ensure that all dependent features and modules are compatible with this change and that there are no unintended side effects.

  • 21-26: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

Adjusting the function declaration for create_file_path could impact how file paths are created and managed within the application. It's important to verify that this change aligns with the intended use cases and that it does not introduce any regressions or compatibility issues with existing code that relies on this function. Additionally, consider reviewing the function's implementation to ensure that it adheres to best practices for error handling, performance, and maintainability.

core/src/location/manager/watcher/mod.rs Show resolved Hide resolved
core/src/location/manager/watcher/ios.rs Show resolved Hide resolved
core/src/location/manager/watcher/ios.rs Show resolved Hide resolved
core/src/location/manager/watcher/ios.rs Show resolved Hide resolved
core/src/location/manager/watcher/ios.rs Show resolved Hide resolved
apps/mobile/scripts/cleanTarget.sh Show resolved Hide resolved
apps/mobile/scripts/cleanTarget.sh Show resolved Hide resolved
apps/mobile/src/screens/LocationOnboarding.tsx Outdated Show resolved Hide resolved
apps/mobile/src/screens/LocationOnboarding.tsx Outdated Show resolved Hide resolved
core/src/location/manager/watcher/android.rs Show resolved Hide resolved
Copy link
Contributor

@fogodev fogodev left a comment

Choose a reason for hiding this comment

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

Just some minor warnings of unused imports, the event handler implementation for iOS seems solid

core/src/location/manager/mod.rs Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 2

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 1c4fbff and db2dc83.
Files selected for processing (4)
  • apps/mobile/src/navigation/index.tsx (2 hunks)
  • core/src/api/locations.rs (1 hunks)
  • core/src/location/manager/mod.rs (16 hunks)
  • core/src/location/manager/watcher/mod.rs (14 hunks)
Files skipped from review as they are similar to previous changes (3)
  • apps/mobile/src/navigation/index.tsx
  • core/src/api/locations.rs
  • core/src/location/manager/watcher/mod.rs
Additional comments: 6
core/src/location/manager/mod.rs (6)
  • 23-23: The addition of tracing::{debug, error} imports aligns with the changes in the file to enhance logging. This is a positive change for better observability and debugging capabilities.
  • 200-200: Adding a debug log at the start of the location manager actor is a good practice for observability, allowing easier tracking of the actor's lifecycle.
  • 229-229: The debug log for sending location management messages improves the visibility of actions taken by the location manager, aiding in debugging and monitoring.
  • 255-255: Similarly, adding a debug log for sending watcher management messages enhances observability and is helpful for debugging purposes.
  • 402-402: Logging the online status of a location when it is being watched is useful for understanding the state of location monitoring. This change is beneficial for debugging and operational monitoring.
  • 552-552: The debug log indicating the stopping of the location manager is crucial for understanding the lifecycle of the manager and troubleshooting any issues related to its termination.

core/src/location/manager/mod.rs Show resolved Hide resolved
core/src/location/manager/mod.rs Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between db2dc83 and edccb8c.
Files selected for processing (3)
  • .vscode/settings.json (1 hunks)
  • apps/mobile/src/navigation/TabNavigator.tsx (1 hunks)
  • apps/mobile/src/navigation/index.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • .vscode/settings.json
  • apps/mobile/src/navigation/TabNavigator.tsx
  • apps/mobile/src/navigation/index.tsx

Copy link
Contributor

@fogodev fogodev left a comment

Choose a reason for hiding this comment

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

Great job

@utkubakir utkubakir added this pull request to the merge queue Feb 29, 2024
Merged via the queue into spacedriveapp:main with commit 4757676 Feb 29, 2024
8 of 10 checks passed
@Rocky43007 Rocky43007 deleted the ios-w-location-watcher branch March 3, 2024 17:40
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

3 participants