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

Makefile, README: remove evm target, add puppeth to table #14886

Merged
merged 3 commits into from
Aug 3, 2017

Conversation

akivab
Copy link
Contributor

@akivab akivab commented Aug 2, 2017

Remove evm, swarm from top-level Makefile (consistency with puppeth) and add puppeth to README

@GitCop
Copy link

GitCop commented Aug 2, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: d50de8d
  • Commits must be prefixed with the package(s) they modify

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

@akivab akivab changed the title Update Makefile puppeth: Update Makefile Aug 2, 2017
Adding puppeth command to top-level Makefile
@akivab akivab force-pushed the puppeth-makefile branch from d50de8d to 3d1db4e Compare August 2, 2017 03:15
@fjl
Copy link
Contributor

fjl commented Aug 2, 2017

Not sure I want this. You can always just run make all, it doesn't take that much longer...

@akivab
Copy link
Contributor Author

akivab commented Aug 2, 2017

If it's a command now available in cmd, should probably be treated same as other commands and updated in README, no?

@fjl
Copy link
Contributor

fjl commented Aug 2, 2017

cmd/puppeth is a command. But not all commands need a dedicated Makefile target. IMHO we have too many targets already. I would approve a PR that deletes the swarm and evm targets for example.

@akivab
Copy link
Contributor Author

akivab commented Aug 2, 2017

Sounds good, thanks. Updated. PTAL

@akivab akivab changed the title puppeth: Update Makefile puppeth: Update Makefile and README Aug 2, 2017
@fjl fjl changed the title puppeth: Update Makefile and README Makefile, README: remove evm, swarm targets, add puppeth info Aug 2, 2017
@lmars
Copy link
Contributor

lmars commented Aug 2, 2017

FYI the Swarm team currently recommends make swarm as the way to build the Swarm binary, I vote to not remove it without an agreed alternative recommendation.

@akivab
Copy link
Contributor Author

akivab commented Aug 2, 2017

I think @fjl suggested using make all for any commands that weren't geth, would that work?

@akivab
Copy link
Contributor Author

akivab commented Aug 3, 2017

Would the recommended action be to close this PR, simply merge in the README changes and leave the Makefile alone, or something else? @fjl @lmars

@fjl
Copy link
Contributor

fjl commented Aug 3, 2017

@lmars I was waiting for your feedback. Let's remove "evm" target but keep "swarm" for now.

@fjl fjl changed the title Makefile, README: remove evm, swarm targets, add puppeth info Makefile, README: remove evm target, add puppeth to table Aug 3, 2017
@fjl fjl merged commit 4371367 into ethereum:master Aug 3, 2017
markya0616 pushed a commit to markya0616/go-ethereum that referenced this pull request Aug 17, 2017
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.

4 participants