Skip to content

Conversation

@quantaf1re
Copy link
Contributor

Changed:

  • Spelling
  • Mentioning that nothing is supposed to be cloned
  • Saying to create files instead of open them when they don't exist
  • Added the comma so this can compile:
    var app = &helloChainApp{
        appStarter,
    }
  • Moving the make.md file to near the end because it requires that the cli is built
  • Corrected file path to x/greeter/client/cli/query.go in client.md

@okwme
Copy link
Contributor

okwme commented Nov 3, 2019

Love the PR thank you!!!

I'm curious to hear @hschoenburg 's thoughts about the re-ordering of the tutorial though. I think he had that order in mind to show what a completely basic chain could do before showing what extra functionality could add.

Might make sense to switch it up, but if so we should keep an eye out for continuity errors like here:
a4cd78c#diff-336e16a561818fb034c8173b054e0deaL106-R107

... Now lets add some functionality of our own making to see how flexible
the Cosmos SDK can be. 

@quantaf1re
Copy link
Contributor Author

Ok, I added another PR without the restructuring

@quantaf1re
Copy link
Contributor Author

Sorry, I meant that I removed the restructuring from this PR and added another PR that includes the restructuring

@quantaf1re quantaf1re changed the title Code/structure/compile fixes Code/compile fixes Nov 3, 2019

install: go.sum
go install $(BUILD_FLAGS) ./cmd/hcd
go install $(BUILD_FLAGS) ./cmd/hccli
Copy link
Contributor

Choose a reason for hiding this comment

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

I do believe this should stay here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The order of the tutorial makes it such that hccli doesn't exist at the point the user make installs this make file - that's why I originally reordered the files, which is now in #185

@tac0turtle tac0turtle merged commit 686265b into cosmos:master Nov 3, 2019
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.

3 participants