Skip to content

Small fix to ensure Assignment field indexes start at 1 - #1604

Merged
stevvooe merged 1 commit into
moby:masterfrom
cyli:fix-dispatcher-proto
Oct 11, 2016
Merged

Small fix to ensure Assignment field indexes start at 1#1604
stevvooe merged 1 commit into
moby:masterfrom
cyli:fix-dispatcher-proto

Conversation

@cyli

@cyli cyli commented Oct 7, 2016

Copy link
Copy Markdown
Contributor

This addresses the comment #1544 (comment).

cc @stevvooe

…1, not 2

Signed-off-by: cyli <ying.li@docker.com>
@codecov-io

Copy link
Copy Markdown

Current coverage is 54.78% (diff: 100%)

Merging #1604 into master will decrease coverage by 0.07%

@@             master      #1604   diff @@
==========================================
  Files            84         84          
  Lines         14174      14174          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           7776       7765    -11   
- Misses         5381       5385     +4   
- Partials       1017       1024     +7   

Sunburst

Powered by Codecov. Last update c7ade46...91bc273

@aaronlehmann

Copy link
Copy Markdown
Contributor

LGTM

@aaronlehmann

Copy link
Copy Markdown
Contributor

ping @docker/core-swarmkit-maintainers - let's merge this soon to minimize compatibility issues

@dperny

dperny commented Oct 11, 2016

Copy link
Copy Markdown
Collaborator

LGTM, though I think if this was just left as 2 it wouldn't make a difference, right? 1-10 are all the same performance-wise, right?

@stevvooe

Copy link
Copy Markdown
Contributor

1-10 are all the same performance-wise, right?

Don't consider performance when making field id selections.

Leaving at 2 is fine, but it always makes you wonder what happened to 1. Is he okay?

@stevvooe

Copy link
Copy Markdown
Contributor

LGTM

@stevvooe
stevvooe merged commit 8a5741e into moby:master Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants