-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
ab44831
to
e7ae1a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small request. Otherwise LGTM.
@@ -359,6 +368,46 @@ private struct FollowModifier: ViewModifier { | |||
} | |||
} | |||
|
|||
private struct RenameModifier: ViewModifier { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that private
here means "private to my project", not "private to this file".
- Recommend moving this to a separate file, or into
FollowUserSheet
- Recommend giving it specific name, perhaps
RenameSheetModifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that private here means "private to my project", not "private to this file".
Based on my testing this isn't true, it means "private to the containing lexical scope" which in this case is the file. private
at the root of a file is equivalent to fileprivate
, no other file can access the type. internal
means "internal to my project".
fileprivate
is now whatprivate
used to be in earlier Swift releases: accessible from the same source file. A declaration marked asprivate
can now only be accessed within the lexical scope it is declared in. Soprivate
is more restrictive thanfileprivate
.
See also: https://forums.swift.org/t/calling-private-methods-from-extensions-in-separate-files/54029
// A.swift
private struct One {}
fileprivate struct Two {}
internal struct Three {}
struct Four {} // implicitly `internal` by default
public struct Five {}
// B.swift
let one: One? = nil // Error: Cannot find 'One' in scope
let two: Two? = nil // Error: Cannot find 'Two' in scope
let three: Three? = nil
let four: Four? = nil
let five: Five? = nil
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved all these sheets into UserProfileView+SheetModifier.swift
and clarified the naming convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3afd564
to
4d0b033
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, with a couple small requests.
Fixes #776
This is a quick way to implement to repurpose follow sheet for renaming users.