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

Generate binaries #121

Merged
merged 10 commits into from Aug 31, 2020
Merged

Generate binaries #121

merged 10 commits into from Aug 31, 2020

Conversation

juliankoh
Copy link
Contributor

@juliankoh juliankoh commented Aug 26, 2020

Motivation

Generate binaries to avoid having to compile manually.

Solution

Open questions

@coveralls
Copy link

coveralls commented Aug 26, 2020

Pull Request Test Coverage Report for Build 4311

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 74.969%

Totals Coverage Status
Change from base Build 4167: 0.0%
Covered Lines: 599
Relevant Lines: 799

💛 - Coveralls

@@ -379,6 +379,7 @@ Global Flags:
* `make test` to run tests
* `make lint` to lint the source code (included generated code)
* `make release` to run one last check before opening a PR
* `make compile version=RELEASE_TAG` to generate binaries
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind adding a small section to the README about using binaries and that a script is in progress?

Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to add that it is a known issue to click download on the release:
hashicorp/terraform#23033

Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out curl bypasses this:

curl -L https://github.com/coinbase/rosetta-cli/releases/download/v0.4.1/rosetta-cli-0.4.1-darwin-10.6-amd64 -o rosetta-cli; chmod +x rosetta-cli;

Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to include this in the README ^^ with a warning to not download on the UI

```
go get github.com/coinbase/rosetta-cli
```

To download the binary directly, run:
```
curl -L https://github.com/coinbase/rosetta-cli/releases/download/v0.4.1/rosetta-cli-0.4.1-darwin-10.6-amd64 -o rosetta-cli; chmod +x rosetta-cli;
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to do a rosetta-cli-{YOUR REQUIRED VERSION HERE}

README.md Outdated
```
go get github.com/coinbase/rosetta-cli
```

To download the binary directly, run:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a binary

README.md Outdated
@@ -29,10 +29,18 @@ Before diving into the CLI, we recommend taking a look at the Rosetta API Docs:
* [Construction API](https://www.rosetta-api.org/docs/construction_api_introduction.html)

## Install
To install `rosetta-cli` by getting the source code, run:
Copy link
Contributor

Choose a reason for hiding this comment

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

not: To install rosetta-cli from source, run:

README.md Outdated
```
go get github.com/coinbase/rosetta-cli
```

To download a binary directly for MacOS, run:
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to list installs for each OS? I was thinking do:

rosetta-cli-{YOUR REQUIRED VERSION HERE}-{YOUR OS HERE}

And then include a link to releases to identify

And then say, for example on mac with version 0.4.1:

curl -L https://github.com/coinbase/rosetta-cli/releases/download/v0.4.1/rosetta-cli-0.4.1-darwin-10.6-amd64 -o rosetta-cli; chmod +x rosetta-cli;

@@ -49,6 +49,9 @@ salus:

release: add-license shorten-lines format test lint salus

compile:
xgo --targets=darwin/*,windows/*,linux/* -out bin/rosetta-cli-$(version) .
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to specify a tag here or do we need to checkout the code we want to compile @juliankoh ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This builds the code locally (as you can see from the . )

We can also build a specific tag but that would be a different command.

@juliankoh juliankoh merged commit 04f3795 into master Aug 31, 2020
@juliankoh juliankoh deleted the julian/bin branch August 31, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants