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

docker, makefile, p2p main #655

Merged
merged 3 commits into from
Mar 14, 2019
Merged

docker, makefile, p2p main #655

merged 3 commits into from
Mar 14, 2019

Conversation

y0sher
Copy link
Contributor

@y0sher y0sher commented Mar 12, 2019

No description provided.

@y0sher y0sher requested a review from gavraz March 12, 2019 21:53
endif
.PHONY: build

hare:
cd cmd/hare ; go build -o $(CURR_DIR)/go-hare; cd ..
ifeq ($(OS),Windows_NT)
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 we should create a func build_path(NAME) and not repeat this if else when not necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

or even use the if once to assign the correct path to $(BIN_DIR)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, but you still need to push the name in the string (see the .exe for example).

cmdp.AddCommands(Cmd)
}

type P2PApp struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not remove the BaseApp? Then the Hare can be composed of P2PApp. Same goes for the SpacemeshApp.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we'll do that it will be a problem compiling the p2p app as a standalone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it has to have a main to do so. and if we'll do it we won't be able to use it embedded in other structs.

Copy link
Contributor

@gavraz gavraz left a comment

Choose a reason for hiding this comment

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

  1. Fix makefile (extract func)
  2. Remove p2p dockerfile

@y0sher
Copy link
Contributor Author

y0sher commented Mar 14, 2019

opened #660 about reducing code duplication in the Makefile

@y0sher y0sher merged commit 402be88 into develop Mar 14, 2019
@y0sher y0sher deleted the p2p_docker_makefile branch June 25, 2019 07:43
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.

None yet

3 participants