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

vBuild() method in message builders #409

Merged
merged 51 commits into from
May 16, 2019
Merged

vBuild() method in message builders #409

merged 51 commits into from
May 16, 2019

Conversation

dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented May 13, 2019

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().

@dmdashenkov dmdashenkov self-assigned this May 13, 2019
@codecov
Copy link

codecov bot commented May 13, 2019

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 armiol May 15, 2019 10:11
@dmdashenkov
Copy link
Contributor Author

@alexander-yevsyukov, PTAL.

@dmdashenkov dmdashenkov changed the title vbuild() method in message builders vBuild() method in message builders May 15, 2019
Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

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
Contributor Author

@alexander-yevsyukov, PTAL again.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

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
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 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