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

Unify worker implementations #19

Merged
merged 1 commit into from
Jan 22, 2025
Merged

Unify worker implementations #19

merged 1 commit into from
Jan 22, 2025

Conversation

tek
Copy link
Contributor

@tek tek commented Jan 21, 2025

Introduce an optional second frontend plugin option that selects the implementation, either the default GHC main function or the local logic with custom session management.

@tek tek requested a review from wavewave January 21, 2025 20:37
Copy link
Collaborator

@wavewave wavewave left a comment

Choose a reason for hiding this comment

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

looks good. For the new rgenerally-typed parameters, please add brief description. Thanks!

@@ -464,11 +464,11 @@ moduleColumns m =
report ::
MonadIO m =>
MVar Log ->
HscEnv ->
Maybe String ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

String is always subject to haddock comment. ;-)

finalizeCache logVar hsc_env target artifacts cache0 = do
finalizeCache ::
MVar Log ->
Maybe String ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

add haddock comment

@@ -49,8 +49,8 @@ runServer socketFile server = do
$ \(conn, _peer) -> void $
forkFinally (server conn) (const $ gracefulClose conn 5000)

initWorker :: FilePath -> [FilePath] -> WorkerId -> IO HandleSet
initWorker ghcPath dbPaths i = do
initWorker :: Bool -> FilePath -> [FilePath] -> WorkerId -> IO HandleSet
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bool is also subject to haddock comment.
especially thisimplCustom meaning is unclear from the name.

Introduce an optional second frontend plugin option that selects the
implementation, either the default GHC main function or the local logic
with custom session management.
@tek tek force-pushed the tek/unify-impls branch from 861b90f to 62bdbb4 Compare January 22, 2025 15:09
@tek tek merged commit a37f8de into main Jan 22, 2025
@tek tek deleted the tek/unify-impls branch January 22, 2025 15:09
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.

2 participants