Convert ManualMainFilesProvider and BSMDelegate to be actors #917
Conversation
joehsieh
force-pushed
the
migrate-queue-to-actor
branch
from
d29e71a to
9144b5c
Compare
| get { lock.sync { _mainFiles } } | ||
| set { lock.sync { _mainFiles = newValue } } | ||
| private final actor ManualMainFilesProvider: MainFilesProvider { | ||
| private var mainFilesDictionary: [DocumentURI: Set<DocumentURI>] |
There was a problem hiding this comment.
One last comment: Dictionary is superfluous here because the type is already a dictionary. I would just name it mainFiles.
There was a problem hiding this comment.
The intention for the renaming is to avoid the confusion between mainFiles: [DocumentURI: Set<DocumentURI>]
mainFiles: Set<DocumentURI> https://github.com/apple/sourcekit-lsp/pull/917/files#diff-4e5dc27e47b431520ccbd13ad2e859517644e3437c73157b2193a7b14ec30570R414
it was a trade-off when i named it, but yeah the data type already convey itself, so let me rename it.
There was a problem hiding this comment.
I think that’s fine. updateMainFiles also is a really short methods and I don’t think it should cause too much confusion.
| init(mainFilesDictionary: [DocumentURI: Set<DocumentURI>]) { | ||
| self.mainFilesDictionary = mainFilesDictionary | ||
| } |
There was a problem hiding this comment.
And I think we can make the parameter unnamed here. Since we are constructing a ManualMainFilesProvider, it’s clear that we are passing the main files, IMO. What do you think?
There was a problem hiding this comment.
agree. let me update the code.
Refactor the code Remove unused code Refine the code
joehsieh
force-pushed
the
migrate-queue-to-actor
branch
from
9144b5c to
ea445cb
Compare
|
@swift-ci Please test |
issue: #868