Resolve TODOs under Shicong's name (#930)

core/build.gradle

@@ -600,7 +600,6 @@ task compileProdJS(type: JavaExec) {
   closureArgs << "--js=${jsDir}/soyutils_usegoog.js"
   closureArgs << "--js=${cssSourceDir}/registrar_bin.css.js"
   closureArgs << "--js=${jsSourceDir}/**.js"
-  // TODO(shicong) Verify the compiled JS file works in Alpha
   closureArgs << "--js=${externsDir}/json.js"
   closureArgs << "--js=${soySourceDir}/**.js"
   args closureArgs

core/src/main/java/google/registry/model/domain/GracePeriod.java

@@ -193,19 +193,6 @@ public class GracePeriod extends GracePeriodBase implements DatastoreAndSqlEntit
     return clone;
   }
 
-  /**
-   * Returns a clone of this {@link GracePeriod} with prepopulated {@link #gracePeriodId} generated
-   * by {@link ObjectifyService#allocateId()}.
-   *
-   * <p>TODO(shicong): Figure out how to generate the id only when the entity is used for Cloud SQL.
-   */
-  @VisibleForTesting
-  public GracePeriod cloneWithPrepopulatedId() {
-    GracePeriod clone = clone(this);
-    clone.gracePeriodId = ObjectifyService.allocateId();
-    return clone;
-  }
-
   /** Entity class to represent a historic {@link GracePeriod}. */
   @Entity(name = "GracePeriodHistory")
   @Table(indexes = @Index(columnList = "domainRepoId"))

core/src/main/java/google/registry/model/domain/secdns/DelegationSignerData.java

@@ -35,7 +35,7 @@ import javax.xml.bind.annotation.XmlType;
  *
  * @see <a href="http://tools.ietf.org/html/rfc5910">RFC 5910</a>
  * @see <a href="http://tools.ietf.org/html/rfc4034">RFC 4034</a>
- *     <p>TODO(shicong): Rename this class to DomainDsData.
+ *     <p>TODO(b/177567432): Rename this class to DomainDsData.
  */
 @Embed
 @XmlType(name = "dsData")

core/src/main/java/google/registry/model/eppcommon/Address.java

@@ -59,8 +59,8 @@ import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
 public class Address extends ImmutableObject implements Jsonifiable {
 
   /** The schema validation will enforce that this has 3 lines at most. */
-  // TODO(shicong): Remove this field after migration. We need to figure out how to generate same
+  // TODO(b/177569726): Remove this field after migration. We need to figure out how to generate
-  // XML from streetLine[1,2,3].
+  // same XML from streetLine[1,2,3].
   @XmlJavaTypeAdapter(NormalizedStringAdapter.class)
   @Transient
   List<String> street;

core/src/main/java/google/registry/model/registrar/Registrar.java

@@ -234,7 +234,7 @@ public class Registrar extends ImmutableObject
    * Unique registrar client id. Must conform to "clIDType" as defined in RFC5730.
    *
    * @see <a href="http://tools.ietf.org/html/rfc5730#section-4.2">Shared Structure Schema</a>
-   *     <p>TODO(shicong): Rename this field to clientId
+   *     <p>TODO(b/177568946): Rename this field to registrarId.
    */
   @Id
   @javax.persistence.Id

core/src/main/java/google/registry/model/tmch/ClaimsListDao.java

@@ -58,7 +58,7 @@ public class ClaimsListDao {
 
   /**
    * Returns the most recent revision of the {@link ClaimsListShard} in Cloud SQL, if it exists.
-   * TODO(shicong): Change this method to package level access after dual-read phase.
+   * TODO(b/177569979): Change this method to package level access after dual-read phase.
    * ClaimsListShard uses this method to retrieve claims list in Cloud SQL for the comparison, and
    * ClaimsListShard is not in this package.
    */

core/src/main/java/google/registry/model/transfer/TransferData.java

@@ -69,7 +69,7 @@ public abstract class TransferData<
   Set<VKey<? extends TransferServerApproveEntity>> serverApproveEntities;
 
   // The following 3 fields are the replacement for serverApproveEntities in Cloud SQL.
-  // TODO(shicong): Add getter/setter for these 3 fields and use them in the application code.
+  // TODO(b/177589157): Make transfer flows work with Cloud SQL.
   @Ignore
   @Column(name = "transfer_gaining_poll_message_id")
   Long gainingTransferPollMessageId;

core/src/main/java/google/registry/persistence/PersistenceModule.java

@@ -351,8 +351,8 @@ public abstract class PersistenceModule {
   @interface BeamPipelineCloudSqlConfigs {}
 
   /** Dagger qualifier for the default Hibernate configurations. */
-  // TODO(shicong): Change annotations in this class to none public or put them in a top level
+  // TODO(b/177568911): Change annotations in this class to none public or put them in a top level
-  //  package
+  // package.
   @Qualifier
   @Documented
   public @interface DefaultHibernateConfigs {}

core/src/main/java/google/registry/persistence/converter/CidrAddressBlockListConverter.java

@@ -20,7 +20,6 @@ import javax.persistence.Converter;
 
 /**
  * JPA {@link AttributeConverter} for storing/retrieving {@code List<CidrAddressBlock>} objects.
- * TODO(shicong): Investigate if we can have one converter for any List type
  */
 @Converter(autoApply = true)
 public class CidrAddressBlockListConverter extends StringListConverterBase<CidrAddressBlock> {

core/src/main/java/google/registry/persistence/transaction/JpaTransactionManagerImpl.java

@@ -76,8 +76,9 @@ public class JpaTransactionManagerImpl implements JpaTransactionManager {
   // EntityManagerFactory is thread safe.
   private final EntityManagerFactory emf;
   private final Clock clock;
-  // TODO(shicong): Investigate alternatives for managing transaction information. ThreadLocal adds
+  // TODO(b/177588434): Investigate alternatives for managing transaction information. ThreadLocal
-  //  an unnecessary restriction that each request has to be processed by one thread synchronously.
+  // adds an unnecessary restriction that each request has to be processed by one thread
+  // synchronously.
   private final ThreadLocal<TransactionInfo> transactionInfo =
       ThreadLocal.withInitial(TransactionInfo::new);
 

core/src/test/java/google/registry/model/history/DomainHistoryTest.java

@@ -221,8 +221,7 @@ public class DomainHistoryTest extends EntityTestCase {
         .setGracePeriods(
             ImmutableSet.of(
                 GracePeriod.create(
-                        GracePeriodStatus.ADD, "domainRepoId", END_OF_TIME, "clientId", null)
+                    GracePeriodStatus.ADD, "domainRepoId", END_OF_TIME, "clientId", null)))
-                    .cloneWithPrepopulatedId()))
         .build();
   }
 

core/src/test/java/google/registry/persistence/transaction/JpaTransactionManagerImplTest.java

@@ -48,8 +48,8 @@ import org.junit.jupiter.api.extension.RegisterExtension;
  * Unit tests for SQL only APIs defined in {@link JpaTransactionManagerImpl}. Note that the tests
  * for common APIs in {@link TransactionManager} are added in {@link TransactionManagerTest}.
  *
- * <p>TODO(shicong): Remove duplicate tests that covered by TransactionManagerTest by refactoring
+ * <p>TODO(b/177587763): Remove duplicate tests that covered by TransactionManagerTest by
- * the test schema.
+ * refactoring the test schema.
  */
 class JpaTransactionManagerImplTest {