DaedalusIPC: Fix isWindows function #834
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Erf. Let's not bother with that now. |
| @@ -214,7 +214,7 @@ readMessage :: Handle -> IO BL.ByteString | |||
| readMessage = if isWindows then windowsReadMessage else posixReadMessage | |||
|
|
|||
| isWindows :: Bool | |||
| isWindows = arch == "windows" | |||
| isWindows = os == "windows" | |||
There was a problem hiding this comment.
Just a thought, but perhaps we can replace this Bool check with an enumeration that includes just the platforms we care about. Perhaps like this:
data Architecture = Windows | Linux | MacOSThen we can have:
readMessage = \case
Windows -> windowsReadMessage
Linux -> posixReadMessage
MacOS -> macOsReadMessage
There was a problem hiding this comment.
For what it's worth here, it's really a local check in one place. I don't think we need much more than this.
KtorZ
approved these changes
Oct 16, 2019
|
bors r+ |
Merge conflict (retrying...) |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 16, 2019
834: DaedalusIPC: Fix isWindows function r=KtorZ a=rvl # Issue Number None. # Overview Fixes a typo in the OS detection code for NodeIPC. # Comments I again looked at the possibility of importing the cardano-shell package for this functionality. But it is pulling in more dependencies. Also the `daedalusIPC` function there had the logging replaced with `putTextLn`. Branch is [rvl/daedalus-ipc-cardano-shell](rvl/daedalus-ipc-fix...rvl/daedalus-ipc-cardano-shell) for reference though. Co-authored-by: Rodney Lorrimar <[email protected]>
Build failed |
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 16, 2019
834: DaedalusIPC: Fix isWindows function r=KtorZ a=rvl # Issue Number None. # Overview Fixes a typo in the OS detection code for NodeIPC. # Comments I again looked at the possibility of importing the cardano-shell package for this functionality. But it is pulling in more dependencies. Also the `daedalusIPC` function there had the logging replaced with `putTextLn`. Branch is [rvl/daedalus-ipc-cardano-shell](rvl/daedalus-ipc-fix...rvl/daedalus-ipc-cardano-shell) for reference though. 840: Make Jörmungandr NetworkLayer use raw blocks r=KtorZ a=Anviking # Issue Number #711 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I changed the constructors in `Cardano.Wallet.Jormungandr.Network` to create NetworkLayers with raw jörmungadnr-specific blocks, and later used `toWLBlock <$> nl`. This way we can later use `toSPBlock <$> nl` for stake-pools. # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> 845: Add forget pending transaction endpoint (Part1) r=KtorZ a=paweljakubas # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #836 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - [x] I have added forget pending transaction endpoint, errors and types - [x] I have updated swagger accordingly # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: Pawel Jakubas <[email protected]>
Build succeeded |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
None.
Overview
Fixes a typo in the OS detection code for NodeIPC.
Comments
I again looked at the possibility of importing the cardano-shell package for this functionality. But it is pulling in more dependencies. Also the
daedalusIPCfunction there had the logging replaced withputTextLn. Branch is rvl/daedalus-ipc-cardano-shell for reference though.