Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

[#1] Add first protocol buffers message and print its serialization to terminal #3

Merged
merged 12 commits into from
Aug 13, 2018

Conversation

TejasSC
Copy link
Contributor

@TejasSC TejasSC commented Aug 7, 2018

Resolves #1
Resolves #2

@TejasSC TejasSC self-assigned this Aug 7, 2018
@TejasSC TejasSC requested a review from chshersh August 7, 2018 03:41
@TejasSC TejasSC changed the title (#1) Add first protocol buffers message and print its serialization to terminal [#1] Add first protocol buffers message and print its serialization to terminal Aug 7, 2018
@chshersh
Copy link
Contributor

chshersh commented Aug 7, 2018

@TejasSC I've looked into your error. Unfortunately, I see the same build error 😞
There's this open issue on Github:

Try for now to build with stack instead. Maybe this can resolve your issue.

@chshersh
Copy link
Contributor

@TejasSC Does it work if you only do something simple with ghcid? Like, printing string Hello, world! into terminal? Without imports and other stuff.

@chshersh
Copy link
Contributor

@TejasSC I've created issue #4. But let's do #4 in separate PR. I will consider this PR done when you're able to print encoded message using ghcid. I see that this is already implemented. Could you paste output in comment here?

@TejasSC
Copy link
Contributor Author

TejasSC commented Aug 13, 2018

Following is after "Registering library for proto-route-0.0.0.." line

GHCi, version 8.2.2: http://www.haskell.org/ghc/ :? for help
[1 of 1] Compiling ProtoExports ( src/ProtoExports.hs, interpreted )
Ok, one module loaded.
SearchRequest {_SearchRequest'query = "test", _SearchRequest'_unknownFields = []}
"\n\EOTtest"

Copy link
Contributor

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

This is really great that current results are possible! Some refactoring is required. But I guess we're moving in the right direction.

CHANGELOG.md Outdated
==========

proto-route uses [PVP Versioning][1].
The change log is available [on GitHub][2].
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's actually useful to keep previous content of changelog file.

LICENSE Outdated

Copyright (c) 2018 Holmusk
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that LICENSE is changed. Could you please return back to previous license?

README.md Outdated
@@ -1,10 +1 @@
# proto-route

[![Hackage](https://img.shields.io/hackage/v/proto-route.svg)](https://hackage.haskell.org/package/proto-route)
[![MIT license](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/holmusk/proto-route/blob/master/LICENSE)
Copy link
Contributor

Choose a reason for hiding this comment

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

The previous content of README file also can be returned back

stack.yaml Outdated
@@ -0,0 +1,7 @@
resolver: lts-11.8
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you could try to use latest resolver from Stackage? I guess we figured out that the problem was not in LTS. So I think you can use newer GHC version. And also you can remove those packages from extra-deps that are in the LTS resolver you're going to use.

app/Main.hs Outdated
-- Value of generated msg, and its serialized representation
execStatement "sr"
execStatement "encodeMessage sr"
stopGhci g
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, let's actually move this chunk of code into library stanza. So Main.hs file will look like this:

import qualified ProtoRoute

main :: IO ()
main = ProtoRoute.main

This will make executable stanza very small (almost no dependencies). And we will put all logic into library. This is better for further modularity.

CHANGELOG.md Outdated
## Unreleased changes
==========
proto-route uses [PVP Versioning][1].
The change log is available [on GitHub][2].
Copy link
Contributor

Choose a reason for hiding this comment

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

[...][1] or [...][2] should point to some links. But links are missing in this file.

@chshersh
Copy link
Contributor

@TejasSC Could you also write lines like

Resolves #1
Resolves #2

in the body of your PR? This will close issues automatically when PR is merged.

CHANGELOG.md Outdated
@@ -2,3 +2,6 @@
==========
proto-route uses [PVP Versioning][1].
The change log is available [on GitHub][2].

[PVP Versioning]: https://pvp.haskell.org/
[on GitHub]: https://github.com/Holmusk/proto-route/blob/master/CHANGELOG.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, this won't work 😞 The initial content of proto-route is the following:

@chshersh chshersh merged commit 0618511 into master Aug 13, 2018
@chshersh chshersh deleted the dev517-protobuf-msg1 branch August 13, 2018 09:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants