Skip to content

Conversation

@langsmith
Copy link

@langsmith langsmith commented Apr 9, 2019

Resolves #951 by adjusting the several annotations which @Guardiola31337 listed in #951.

@osana , I don't believe there are any other downstream changes that need to happen based on these annotation updates, but if so, please let me know.

@langsmith langsmith self-assigned this Apr 9, 2019
@langsmith langsmith requested a review from osana April 9, 2019 01:54
@langsmith langsmith force-pushed the ls-fix-banner-MAS-models-to-match-directions branch from d9fdacc to db0eae9 Compare April 9, 2019 01:59
@langsmith langsmith force-pushed the ls-fix-banner-MAS-models-to-match-directions branch from db0eae9 to efa6ea0 Compare April 9, 2019 02:04
@osana
Copy link
Contributor

osana commented Apr 9, 2019

👍 I am guessing once tests are fixed that would be good to go

@langsmith
Copy link
Author

👍 I am guessing once tests are fixed that would be good to go

Yea, had to fix one test

@langsmith langsmith merged commit f50720f into master Apr 9, 2019
@langsmith langsmith deleted the ls-fix-banner-MAS-models-to-match-directions branch April 9, 2019 02:25
.primary(BannerText.builder().build())
.secondary(BannerText.builder().build())
.primary(BannerText.builder().text("Banner primary sample text").build())
.secondary(BannerText.builder().text("Banner secondary sample text").build())
Copy link
Contributor

Choose a reason for hiding this comment

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

If secondary is @Nullable, this is not needed right? I mean, passing a null value (as before) should work too.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for checking in @Guardiola31337 . Looks like null is fine as a parameter.

Screen Shot 2019-04-10 at 2 01 08 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah and we'd be implicitly "testing" that secondary could be @Nullable, thanks for confirming @langsmith

@osana osana mentioned this pull request Apr 11, 2019
16 tasks
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.

3 participants