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

Use abstract class instead of sealed classes for StringFormat #6334

Merged
merged 2 commits into from
Oct 3, 2024

Conversation

rlazo
Copy link
Collaborator

@rlazo rlazo commented Oct 1, 2024

sealed classes have nice properties but they don't play well with API backward compatibility. Adding a new class to a sealed class set is a breaking change since they require exhaustive switching.

`sealed` classes have nice properties but they don't play well with
API backward compatibility. Adding a new class to a `sealed` class set
is a breaking change since they require exhaustive switching.
@rlazo rlazo requested a review from daymxn October 1, 2024 20:52
Copy link
Contributor

github-actions bot commented Oct 1, 2024

Javadoc Changes:
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/vertexai/type/StringFormat.Custom.html	2024-10-01 20:59:21.495756180 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/vertexai/type/StringFormat.Custom.html	2024-10-01 20:57:12.258729977 +0000
@@ -44,48 +44,17 @@
         <tbody class="list">
           <tr>
             <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;format)</code></div>
+              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;value)</code></div>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive" id="inhmethods">
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Inherited fields</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td><devsite-expandable><span class="expand-control">From <a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html">com.google.firebase.vertexai.type.StringFormat</a></span>
-              <div class="devsite-table-wrapper">
-                <table class="responsive">
-                  <colgroup>
-                    <col width="40%">
-                    <col>
-                  </colgroup>
-                  <tbody class="list">
-                    <tr>
-                      <td><code>final @<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a></code></td>
-                      <td>
-                        <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a></code></div>
-                      </td>
-                    </tr>
-                  </tbody>
-                </table>
-              </div>
-</devsite-expandable>            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
     <div class="list">
       <h2>Public constructors</h2>
       <div class="api-item"><a name="Custom-kotlin.String-"></a><a name="custom"></a>
         <h3 class="api-name" id="Custom(kotlin.String)">Custom</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;format)</pre>
+        <pre class="api-signature no-pretty-print">public&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;value)</pre>
       </div>
     </div>
   </body>
--- /home/runner/diff/original/firebase-kotlindoc/android/com/google/firebase/vertexai/type/StringFormat.html	2024-10-01 20:59:21.492756203 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/android/com/google/firebase/vertexai/type/StringFormat.html	2024-10-01 20:57:12.254730008 +0000
@@ -8,7 +8,7 @@
     <div id="metadata-info-block"></div>
     <h1>StringFormat</h1>
     <p>
-      <pre>public class <a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html">StringFormat</a></pre>
+      <pre>public abstract class <a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html">StringFormat</a></pre>
     </p>
     <div class="devsite-table-wrapper"><devsite-expandable><span class="expand-control jd-sumtable-subclasses">Known direct subclasses
         <div class="showalways" id="subclasses-direct"><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.Custom.html">StringFormat.Custom</a></div>
@@ -52,61 +52,6 @@
         </tbody>
       </table>
     </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive">
-        <colgroup>
-          <col width="40%">
-          <col>
-        </colgroup>
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Public fields</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td><code>final @<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a></code></td>
-            <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a></code></div>
-            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive">
-        <colgroup>
-          <col width="40%">
-          <col>
-        </colgroup>
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Protected constructors</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td>
-              <div><code><a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html#StringFormat(kotlin.String)">StringFormat</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;value)</code></div>
-            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
-    <div class="list">
-      <h2>Public fields</h2>
-      <div class="api-item"><a name="getValue()"></a><a name="setValue()"></a><a name="getValue--"></a><a name="setValue--"></a>
-        <h3 class="api-name" id="value()">value</h3>
-        <pre class="api-signature no-pretty-print">public&nbsp;final&nbsp;@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a></pre>
-      </div>
-    </div>
-    <div class="list">
-      <h2>Protected constructors</h2>
-      <div class="api-item"><a name="StringFormat-kotlin.String-"></a><a name="stringformat"></a>
-        <h3 class="api-name" id="StringFormat(kotlin.String)">StringFormat</h3>
-        <pre class="api-signature no-pretty-print">protected&nbsp;<a href="/docs/reference/android/com/google/firebase/vertexai/type/StringFormat.html#StringFormat(kotlin.String)">StringFormat</a>(@<a href="https://developer.android.com/reference/kotlin/androidx/annotation/NonNull.html">NonNull</a> <a href="https://developer.android.com/reference/kotlin/java/lang/String.html">String</a>&nbsp;value)</pre>
-      </div>
-    </div>
   </body>
 </html>
 
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html	2024-10-01 20:59:21.489756226 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html	2024-10-01 20:57:12.251730030 +0000
@@ -44,48 +44,17 @@
         <tbody class="list">
           <tr>
             <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(format:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</code></div>
+              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(value:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</code></div>
             </td>
           </tr>
         </tbody>
       </table>
     </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive" id="inhmethods">
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Inherited properties</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td><devsite-expandable><span class="expand-control">From <a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html">com.google.firebase.vertexai.type.StringFormat</a></span>
-              <div class="devsite-table-wrapper">
-                <table class="responsive">
-                  <colgroup>
-                    <col width="40%">
-                    <col>
-                  </colgroup>
-                  <tbody class="list">
-                    <tr>
-                      <td><code><a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a></code></td>
-                      <td>
-                        <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a></code></div>
-                      </td>
-                    </tr>
-                  </tbody>
-                </table>
-              </div>
-</devsite-expandable>            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
     <div class="list">
       <h2>Public constructors</h2>
       <div class="api-item"><a name="Custom-kotlin.String-"></a><a name="custom"></a>
         <h3 class="api-name" id="Custom(kotlin.String)">Custom</h3>
-        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(format:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</pre>
+        <pre class="api-signature no-pretty-print"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html#Custom(kotlin.String)">Custom</a>(value:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</pre>
       </div>
     </div>
   </body>
--- /home/runner/diff/original/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/StringFormat.html	2024-10-01 20:59:21.486756249 +0000
+++ /home/runner/diff/modified/firebase-kotlindoc/kotlin/com/google/firebase/vertexai/type/StringFormat.html	2024-10-01 20:57:12.248730053 +0000
@@ -8,7 +8,7 @@
     <div id="metadata-info-block"></div>
     <h1>StringFormat</h1>
     <p>
-      <pre>sealed class <a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html">StringFormat</a></pre>
+      <pre>abstract class <a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html">StringFormat</a></pre>
     </p>
     <div class="devsite-table-wrapper"><devsite-expandable><span class="expand-control jd-sumtable-subclasses">Known direct subclasses
         <div class="showalways" id="subclasses-direct"><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.Custom.html">StringFormat.Custom</a></div>
@@ -52,61 +52,6 @@
         </tbody>
       </table>
     </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive">
-        <colgroup>
-          <col width="40%">
-          <col>
-        </colgroup>
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Protected constructors</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html#StringFormat(kotlin.String)">StringFormat</a>(value:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</code></div>
-            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
-    <div class="devsite-table-wrapper">
-      <table class="responsive">
-        <colgroup>
-          <col width="40%">
-          <col>
-        </colgroup>
-        <thead>
-          <tr>
-            <th colspan="100%"><h3>Public properties</h3></th>
-          </tr>
-        </thead>
-        <tbody class="list">
-          <tr>
-            <td><code><a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a></code></td>
-            <td>
-              <div><code><a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a></code></div>
-            </td>
-          </tr>
-        </tbody>
-      </table>
-    </div>
-    <div class="list">
-      <h2>Protected constructors</h2>
-      <div class="api-item"><a name="StringFormat-kotlin.String-"></a><a name="stringformat"></a>
-        <h3 class="api-name" id="StringFormat(kotlin.String)">StringFormat</h3>
-        <pre class="api-signature no-pretty-print">protected&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html#StringFormat(kotlin.String)">StringFormat</a>(value:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a>)</pre>
-      </div>
-    </div>
-    <div class="list">
-      <h2>Public properties</h2>
-      <div class="api-item"><a name="getValue()"></a><a name="setValue()"></a><a name="getValue--"></a><a name="setValue--"></a>
-        <h3 class="api-name" id="value()">value</h3>
-        <pre class="api-signature no-pretty-print">val&nbsp;<a href="/docs/reference/kotlin/com/google/firebase/vertexai/type/StringFormat.html#value()">value</a>:&nbsp;<a href="https://kotlinlang.org/api/latest/jvm/stdlib/kotlin/-string/index.html">String</a></pre>
-      </div>
-    </div>
   </body>
 </html>
 

@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

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Unit Test Results

  16 files   -   32    16 suites   - 32   13s ⏱️ - 1m 10s
107 tests  - 377  107 ✔️  - 377  0 💤 ±0  0 ±0 
214 runs   - 754  214 ✔️  - 754  0 💤 ±0  0 ±0 

Results for commit 96668a9. ± Comparison against base commit bb5a4d4.

This pull request removes 484 and adds 107 tests. Note that renamed tests count towards both.
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ descriptor should have expected values
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ deserialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueSerializerUnitTest ‑ serialize() should throw UnsupportedOperationException
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Boolean) creates an object with the expected value
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Double) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(List) creates an object with the expected value (normal cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (edge cases)
com.google.firebase.dataconnect.AnyValueUnitTest ‑ constructor(Map) creates an object with the expected value (normal cases)
…
com.google.firebase.vertexai.SchemaTests ‑ basic schema declaration
com.google.firebase.vertexai.SchemaTests ‑ full schema declaration
com.google.firebase.vertexai.StreamingSnapshotTests ‑ citation parsed correctly
com.google.firebase.vertexai.StreamingSnapshotTests ‑ empty content
com.google.firebase.vertexai.StreamingSnapshotTests ‑ http errors
com.google.firebase.vertexai.StreamingSnapshotTests ‑ image rejected
com.google.firebase.vertexai.StreamingSnapshotTests ‑ invalid api key
com.google.firebase.vertexai.StreamingSnapshotTests ‑ invalid json
com.google.firebase.vertexai.StreamingSnapshotTests ‑ long reply
com.google.firebase.vertexai.StreamingSnapshotTests ‑ malformed content
…

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.

This is a good example of something that doesn't need to be a sealed class, LGTM

@rlazo rlazo merged commit 87b6d12 into main Oct 3, 2024
23 checks passed
@rlazo rlazo deleted the rl.vertex.no.sealed.classes branch October 3, 2024 15:02
@rlazo rlazo added this to the vertexai-ga milestone Oct 3, 2024
@firebase firebase locked and limited conversation to collaborators Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants