-
Notifications
You must be signed in to change notification settings - Fork 5.3k
api: Put Event::TimeSystem& into API and plumb that instead of TimeSystem in various places. #5660
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
Merged
jmarantz
merged 30 commits into
envoyproxy:master
from
jmarantz:plumb-time-source-thru-api
Feb 5, 2019
Merged
Changes from all commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
3f92f52
Put Event::TimeSystem& into API and plumb that instead of TimeSystem …
jmarantz 991f7e6
Remove TimeSystem arg from DispatcherImpl.
jmarantz b9add27
use a Global to manage time-systems to avoid having to plumb them thr…
jmarantz 53b83d9
Remove TimeSystem argument to createApiForTest.
jmarantz a1234d7
format
jmarantz f256e6c
Fix more compiler issues.
jmarantz d1ba8e6
Simplifications.
jmarantz d5dbd0f
remove extra include.
jmarantz dbc6f3b
some delegation hacks to reduce the amount of indirection.
jmarantz 3f7396e
format
jmarantz 0deb25b
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 5360c97
Compiles and most tests work.
jmarantz f4088b6
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 5fad5d6
remove non-essential change.
jmarantz 98fe7cc
More diff reduction.
jmarantz 9e90ba8
remove more superfluous changes.
jmarantz c31f6ce
Further diff reduction.
jmarantz 6866490
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 9605da6
Merge branch 'master' into plumb-time-source-thru-api
jmarantz cf51895
Update API instantiation reference
jmarantz 741a021
Merge branch 'master' into plumb-time-source-thru-api
jmarantz eec742d
Merge branch 'master' into plumb-time-source-thru-api
jmarantz ca1c0a8
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 53394d6
make API accessors consistently inlined
jmarantz b08d473
Address reivew comments, and provide a variant of createApiForTest wi…
jmarantz 8e3ccb6
Merge branch 'master' into plumb-time-source-thru-api
jmarantz f9ea8eb
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 1aae0cc
Remove superfluous test_time_. Make test-name unique.
jmarantz b4817b2
Merge branch 'master' into plumb-time-source-thru-api
jmarantz 2b5a16e
Add TODO for changing API interface to TimeSource.
jmarantz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per discussion, WDYT about just making this return a
TimeSourcein this PR? We can still take aTimeSystemin the constructor and not change anything else until a follow up, but this would get us closer to where we want to be?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that would expand this PR significantly because there are a few places in the PR where an existing timeSystem() interface is implemented using API::timeSystem(). If that disappeared I'd have to change a bunch of call-sites. This would be trivial but it would make the PR a lot larger, and there is some semantic content here. So I'd prefer to do that in a separate PR. That OK?
I have this practice of trying to make semantically interesting PRs as small as possible, and letting trivial renaming or type-change PRs go large if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure SGTM. I think we are on the same page and heading in the right direction so 👍