Skip to content

vBuild() method in message builders - #409

Merged
dmdashenkov merged 51 commits into
masterfrom
builder-vbuild
May 16, 2019
Merged

vBuild() method in message builders#409
dmdashenkov merged 51 commits into
masterfrom
builder-vbuild

Conversation

@dmdashenkov

@dmdashenkovdmdashenkov commented May 13, 2019

Copy link
Copy Markdown
Contributor

This PR introduces the io.spine.protobuf.ValidatingBuilder interface—the substitution for the old io.spine.validate.ValidatingBuilder. The new interface is implemented directly in the generated message builders.

The correct way to use message builders now is:

  • use vBuild() method in most cases; it builds the message and performs validation;
  • to avoid validation, use buildPartial();
  • usage of build() is discoureged.

The old UseVBuilders ErrorProne check is now disabled, as it is not recommended to use VBuilders anymore. Instead, we introduce the UseVBuild check, which makes sure users avoid calling build().

@dmdashenkovdmdashenkov self-assigned this May 13, 2019
@codecov

codecovBot commented May 13, 2019

Copy link
Copy Markdown

Codecov Report

Merging #409 into master will increase coverage by 0.75%.
The diff coverage is 68.09%.

@@ Coverage Diff @@## master #409 +/- ##
===========================================
+ Coverage 81.34% 82.1% +0.75% - Complexity 2732 2769 +37 
===========================================
Files 429 432 +3 Lines 10407 10451 +44 Branches 610 619 +9 ===========================================
+ Hits 8466 8581 +115 + Misses 1759 1684 -75 - Partials 182 186 +4

@dmdashenkov
dmdashenkov requested a review from armiolMay 15, 2019 10:11
@dmdashenkov

Copy link
Copy Markdown
ContributorAuthor

@alexander-yevsyukov, PTAL.

@dmdashenkovdmdashenkov changed the title vbuild() method in message buildersvBuild() method in message buildersMay 15, 2019

@alexander-yevsyukovalexander-yevsyukov left a comment

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.

Please address coverage as we discussed vocally.

@dmdashenkov

Copy link
Copy Markdown
ContributorAuthor

@alexander-yevsyukov, PTAL again.

@alexander-yevsyukovalexander-yevsyukov left a comment

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.

LGTM, with one small request.

* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*/

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.

Please document the package.

Sign up for freeto 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

@dmdashenkov@alexander-yevsyukov