Skip to content

Stabilize the api proto descriptors in swarmkit#2349

Merged
nishanttotla merged 3 commits intomoby:masterfrom
tossmilestone:stabilize-api
Aug 15, 2017
Merged

Stabilize the api proto descriptors in swarmkit#2349
nishanttotla merged 3 commits intomoby:masterfrom
tossmilestone:stabilize-api

Conversation

@tossmilestone
Copy link
Copy Markdown
Contributor

@tossmilestone tossmilestone commented Aug 11, 2017

Use the API stability tool of protobuild to stabilize the proto descriptors in swarmkit. It will generate:

  • api/api.pb.txt for api package

Signed-off-by: He Xiaoxi tossmilestone@gmail.com

Signed-off-by: He Xiaoxi <tossmilestone@gmail.com>
Signed-off-by: He Xiaoxi <tossmilestone@gmail.com>
@tossmilestone
Copy link
Copy Markdown
Contributor Author

@stevvooe PTAL, thanks!

@tossmilestone tossmilestone force-pushed the stabilize-api branch 8 times, most recently from 775003a to 086831b Compare August 11, 2017 16:27
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 11, 2017

Codecov Report

Merging #2349 into master will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #2349     +/-   ##
=========================================
- Coverage    60.1%   60.01%   -0.1%     
=========================================
  Files         128      128             
  Lines       26177    26177             
=========================================
- Hits        15734    15709     -25     
- Misses       9045     9070     +25     
  Partials     1398     1398

Comment thread Protobuild.toml Outdated

[[descriptors]]
prefix = "github.com/docker/swarmkit/protobuf/plugin"
target = "protobuf/plugin/plugin.pb.txt"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need to stabilize the plugin API, they are only used here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok,I will remove it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stevvooe updated, please review again

@stevvooe
Copy link
Copy Markdown
Contributor

LGTM

Copy link
Copy Markdown
Contributor

@nishanttotla nishanttotla left a comment

Choose a reason for hiding this comment

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

@tossmilestone @stevvooe do we need to include the api/api.pb.txt file in the repo?

@stevvooe
Copy link
Copy Markdown
Contributor

@nishanttotla That is the entire point of the PR. ;) The idea is that symbol changes in api/api.pb.txt will identify possible breaking API changes.

Please see the README in containerd. That explains it more thoroughly. Perhaps, this PR should include a similar readme.

@tossmilestone
Copy link
Copy Markdown
Contributor Author

tossmilestone commented Aug 15, 2017

@stevvooe I will add a similar README in containerd later to clarify how to use it.

Signed-off-by: He Xiaoxi <tossmilestone@gmail.com>
Copy link
Copy Markdown
Contributor

@nishanttotla nishanttotla left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @tossmilestone for adding the README.

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