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.

Sign up for GitHub

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

Jump to bottom

Revamp Schema declaration #6258

Merged
merged 17 commits into from
Sep 19, 2024
Merged

Revamp Schema declaration #6258

merged 17 commits into from
Sep 19, 2024

Conversation

rlazo
Copy link
Collaborator

@rlazo rlazo commented Sep 13, 2024 •
edited
Loading

b/365820023

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 13, 2024 •
edited
Loading

Release note changes

No release note changes were detected. If you made changes that should be
present in the next release, ensure you've added an entry in the appropriate
CHANGELOG.md file(s).

@rlazo rlazo mentioned this pull request Sep 13, 2024
Merged
@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Sep 13, 2024 •
edited
Loading

Coverage Report 1

Affected Products

No changes between base commit (d153670) and merge commit (eb29b01).

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/ObVUoobP7I.html

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 13, 2024 •
edited
Loading

Unit Test Results

  16 files  +2    16 suites  +2   15s ⏱️ -1s
108 tests +2  108 ✔️ +2  0 💤 ±0  0 ❌ ±0 
216 runs  +4  216 ✔️ +4  0 💤 ±0  0 ❌ ±0 

Results for commit 438b76c. ± Comparison against base commit d153670.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Sep 13, 2024 •
edited
Loading

Size Report 1

Affected Products

  • firebase-vertexai

    TypeBase (d153670)Merge (eb29b01)Diff
    aar507 kB487 kB-20.3 kB (-4.0%)
    apk (release)9.32 MB9.30 MB-19.8 kB (-0.2%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/YUW5Ec7c2v.html

@rlazo rlazo marked this pull request as ready for review September 16, 2024 14:36
@rlazo rlazo changed the title [DRAFT] Revamp Schema declaration Revamp Schema declaration Sep 16, 2024
rachelsaunders
rachelsaunders requested changes Sep 16, 2024
View reviewed changes
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/FunctionDeclaration.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai/type/Schema.kt Outdated Show resolved Hide resolved
rlazo and others added 2 commits September 16, 2024 16:44
@rlazo
Use null instead of StringFormat.None()
0bbb93f
@rlazo @rachelsaunders
Apply suggestions from code review
e03f1e7 
Co-authored-by: rachelsaunders <52258509+rachelsaunders@users.noreply.github.com>
@rlazo @rachelsaunders
Update firebase-vertexai/src/main/kotlin/com/google/firebase/vertexai…
4699edb 
…/type/FunctionDeclaration.kt

Co-authored-by: rachelsaunders <52258509+rachelsaunders@users.noreply.github.com>
@rlazo
Copy link
Collaborator Author

rlazo commented Sep 17, 2024

@daymxn @davidmotson friendly ping

@rlazo
Copy link
Collaborator Author

rlazo commented Sep 17, 2024

@daymxn @davidmotson friendly ping

daymxn
daymxn approved these changes Sep 18, 2024
View reviewed changes
Copy link
Member

@daymxn daymxn left a comment

Choose a reason for hiding this comment

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

Leaving offline discussion here for context:

Ideally we'd use a more kotliny approach, but since that would require overhead we don't currently have- this approach should be fine for now; ensuring we align with other SDKs.

@rlazo rlazo merged commit 3c842c7 into main Sep 19, 2024
28 of 30 checks passed
@rlazo rlazo deleted the rl.schema.revamp branch September 19, 2024 18:57
@rlazo rlazo added this to the vertexai-ga milestone Sep 21, 2024
@firebase firebase locked and limited conversation to collaborators Oct 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rachelsaunders rachelsaunders rachelsaunders approved these changes

@daymxn daymxn daymxn approved these changes

@davidmotson davidmotson Awaiting requested review from davidmotson

Assignees
No one assigned
Labels
size/XL
Projects
None yet
Milestone
vertexai-ga
Development

Successfully merging this pull request may close these issues.
4 participants
@rlazo @google-oss-bot @daymxn @rachelsaunders