Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Feature/issue 738 - Support for Short and Full UUID App ID #802

Merged
merged 8 commits into from
Aug 27, 2018

Conversation

bilal-alsharifi
Copy link
Contributor

@bilal-alsharifi bilal-alsharifi commented Jun 28, 2018

Fixes #738

This PR is ready for review.

Risk

This PR makes minor API changes.

Testing Plan

  • Updated the current unit tests to work with the new fullAppID field in RegisterAppInterface.

Summary

This PR adds a new fullAppID parameter to RegisterAppInterface RPC as described in the proposal.

CLA

@codecov-io
Copy link

codecov-io commented Jun 28, 2018

Codecov Report

Merging #802 into develop will increase coverage by 0.02%.
The diff coverage is 69.56%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #802      +/-   ##
=============================================
+ Coverage       43.8%   43.83%   +0.02%     
- Complexity      3038     3057      +19     
=============================================
  Files            383      383              
  Lines          17795    17849      +54     
  Branches        1750     1753       +3     
=============================================
+ Hits            7795     7824      +29     
- Misses          9672     9698      +26     
+ Partials         328      327       -1
Impacted Files Coverage Δ Complexity Δ
...martdevicelink/proxy/rpc/RegisterAppInterface.java 90.27% <69.56%> (-9.73%) 35 <6> (+4)
...ain/java/com/smartdevicelink/proxy/rpc/Choice.java 84.61% <0%> (-15.39%) 32% <0%> (+15%)
...smartdevicelink/encoder/VirtualDisplayEncoder.java 24.71% <0%> (-4.6%) 8% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae07b1a...5e87a24. Read the comment docs.

@bilal-alsharifi bilal-alsharifi changed the title [WIP] Feature/issue 738 - Support for Short and Full UUID App ID Feature/issue 738 - Support for Short and Full UUID App ID Jun 28, 2018
public static final String GENERAL_APP_ID = "123e4567e8";
public static final String GENERAL_FULL_APP_ID = "123e4567-e89b-12d3-a456-426655440000";


Copy link
Contributor

Choose a reason for hiding this comment

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

please remove extra white space

Bilal added 2 commits August 23, 2018 11:11
# Conflicts:
#	sdl_android/src/androidTest/java/com/smartdevicelink/test/Test.java

/**
* Sets a unique ID, which an app will be given when approved
*
Copy link
Member

Choose a reason for hiding this comment

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

Please add a note that this will automatically parse the full id into the smaller app id and set it


@Override
public void format(Version rpcVersion, boolean formatParams) {
if (getFullAppID() == null){
Copy link
Member

Choose a reason for hiding this comment

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

Please add a check here if(rpcVersion == null || rpcVersion.getMarjor() >= 5)

@joeygrover joeygrover merged commit a4788cc into develop Aug 27, 2018
@joeygrover joeygrover deleted the feature/issue_738 branch August 27, 2018 17:55
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants