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

Update READMEs to document dep-based workflow. #606

Merged
merged 1 commit into from
Feb 2, 2018

Conversation

mandrigin
Copy link
Contributor

Updated workflow for upstream rebasing with dep.

Closes #578

Copy link
Contributor

@themue themue left a comment

Choose a reason for hiding this comment

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

Only minor formal wishes.


### From very scratch
Use this method if you're up to nuke forked repo for some reason and apply patches from scratch:
#### I. In `go-ethereum fork`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would write

I. In our fork at /status-im/go-ethereum


## Manually
This method should be used only while `dep` tool workflow is not set up.
#### II. In `status-go` repository
Copy link
Contributor

Choose a reason for hiding this comment

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

Also here status-go is a name, no code. So simply

II. In status-go repository

- [`0006-latest-cht.patch`](./0006-latest-cht.patch) – updates CHT root hashes, should be updated regularly to keep sync fast, until proper Trusted Checkpoint sync is not implemented as part of LES/2 protocol.
- [`0007-README.patch`](./0007-README.patch) — update upstream README.md.
- [`0008-tx-pool-nonce.patch`](./0008-tx-pool-nonce.patch) - On GetTransactionCount request with PendingBlockNumber get the nonce from transaction pool
- [`0010-geth-17-fix-npe-in-filter-system.patch`](./0010-geth-17-fix-npe-in-filter-system.patch) - Temp patch for 1.7.x to fix a NPE in the filter system
Copy link
Contributor

Choose a reason for hiding this comment

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

Not yet sure if "17" is a good abbreviation for version 1.7.x. But that's more a topic about naming schemes to manage different versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I agree.
hope this patch will go away soon anyway :)

Copy link
Contributor

@adambabik adambabik left a comment

Choose a reason for hiding this comment

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

LGTM! Some copy suggestions.

```
# merge upstream release branch into local master
git pull git@github.com:ethereum/go-ethereum.git release/1.7:master
git pull git@github.com:ethereum/go-ethereum.git release/1.7:develop
Copy link
Contributor

Choose a reason for hiding this comment

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

We should put a comment saying that upstream release branch depends on the current stable geth version we use. Currently, it's 1.7.x.


1. Update vendored `go-ethereum` (note that we use upstream's address there, the link to our fork is in `Gopkg.toml`)
Copy link
Contributor

Choose a reason for hiding this comment

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

[...] the link to our fork is in [...] maybe [...] we override the download link to our fork address in [...]

@mandrigin mandrigin merged commit 6728dcf into develop Feb 2, 2018
@mandrigin
Copy link
Contributor Author

Merging before CI passes because there are no code changes.
@adambabik @themue thanks for your feedback, fixed the points you noted.

@mandrigin mandrigin deleted the igorm/578-update-readme branch February 2, 2018 17:44
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