Code review comment for lp:~diegosarmentero/ubuntuone-client/darwin2-fsevents

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

> The implementation in add_watches_to_udf_ancestors is *very* specific to
> windows, so it needs to be moved to windows.py
>

Moved to windows, we will need to implement later for darwin something here, but not just returning True as in windows.

> ---
>
> FILESYSTEM_MONITOR_MASK gets broken in this branch because
> filesystem_monitor_mask seems to always be None.
>

This is not broken, the mask is assigned in windows.py, and for darwin i'm using it as None, so i can share more common code in common.py.

> ---
>
> All of this would break on darwin:
> is_valid_syncdaemon_path = None
> is_valid_os_path = None
> os_path = windowspath = None
>
> So everything under:
> elif sys.platform == "darwin":
>
> should come on a following branch.
>
> ---
>
> Please rename "reference" to something less vague, and move each definition to
> the platform specific files.

Done

« Back to merge proposal