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

horizon: refactor and improve requestid package #889

Merged
merged 3 commits into from
Feb 14, 2019

Conversation

howardtw
Copy link
Contributor

@howardtw howardtw commented Feb 14, 2019

  1. requestid package originally used type int and value 0 as its context key, which may potentially collide with other context keys. This PR uses an unexported type for its context key.
  2. requestid package was in go/support/context, but it is chi specific and only used by horizon. This PR moves it into horizon/internal.
  3. idiomatic go style doesn't really use main.go for non-command line source. This PR renames the files to context.go and context_test.go.
  4. This PR also renames functions in the file for improved readability.

@howardtw howardtw self-assigned this Feb 14, 2019
Copy link
Member

@ire-and-curses ire-and-curses left a comment

Choose a reason for hiding this comment

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

LGTM

@howardtw howardtw merged commit 6328ae8 into stellar:master Feb 14, 2019
@howardtw howardtw deleted the reqid-update branch February 20, 2019 00:21
oryband added a commit to kinecosystem/go that referenced this pull request Feb 20, 2019
…ate-to-stellar-head

* stellar/master: (23 commits)
  Fix reference to non-existent bash variable
  add account streaming to the api doc (stellar#902)
  build: add Envelope method to tx builder (stellar#716)
  Use `routePattern` variable in `logEndOfRequest` (stellar#893)
  horizon: update changelog (stellar#888)
  horizon: refactor and improve requestid package (stellar#889)
  Fix name of `price` field on Trade (stellar#780)
  Fix case of `price` keys in example response (stellar#779)
  horizon.cmd: fix migration checks (stellar#886)
  horizon/cmd: point to current horizon repo changelog on migration cmd error (stellar#882)
  horizon: remove redundant return param from NewApp() (stellar#885)
  horizon/cmd: require at least one argument for rootCmd (stellar#879)
  horizon: consolidate middlewares (stellar#880)
  horizon: move cmd files into horizon/cmd (stellar#877)
  horizon/actions: have JSON() and Raw() return an error (stellar#873)
  horizon/actions: improve streaming error handling (stellar#856)
  travis: add gofmt and fix govet (stellar#865)
  horizon/actions: fix slice looping for offers in an account (stellar#837)
  Set default value when client data not present (stellar#860)
  Update Horizon 0.16.0 CHANGELOG with db migration notes (stellar#859)
  ...
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.

2 participants