jdaugherty commented on code in PR #15715: URL: https://github.com/apache/grails-core/pull/15715#discussion_r3363102169
########## grails-core/src/main/groovy/org/grails/util/TypeConverters.java: ########## @@ -0,0 +1,274 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.util; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.List; + +import grails.util.GrailsStringUtils; + +/** + * Internal helpers that convert a raw {@code Object} value to a target type without allocating + * an intermediate map. These power the type conversion methods on {@code TypeConvertingMap} and + * the servlet attribute extensions, keeping the conversion logic in a single place. + * + * <p>This is an internal class and is not part of the public Grails API. Review Comment: If this is an internal class, we shouldn't place this in org.grails.util. The `grails.*` package was previously reserved for these type of classes. I'd suggest we take this opportunity to move it under the apache namespace, so I would put this under `org.apache.grails.core.internal.util` Reasoning being: * Base package across the entire project should represent our project - i.e. `org.apache.grails` * Package names should be unique per gradle project (JMS compliance) - hence, the `.core` addition * `internal` to indicate it shouldn't be used externally, this mirrors the old `grails.*` namespace * `util` since it was extracted from existing `util` package code ########## grails-web-core/src/main/groovy/org/grails/web/servlet/FlashScopeExtension.groovy: ########## @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.web.servlet + +import groovy.transform.CompileStatic + +import org.grails.util.TypeConverters + +import grails.web.mvc.FlashScope + +/** + * An extension that adds type conversion methods to the {@link FlashScope} interface + * + * @since 7.2 Review Comment: 8.0 ########## grails-core/src/main/groovy/org/grails/util/TypeConverters.java: ########## @@ -0,0 +1,274 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.util; + +import java.text.ParseException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.List; + +import grails.util.GrailsStringUtils; + +/** + * Internal helpers that convert a raw {@code Object} value to a target type without allocating + * an intermediate map. These power the type conversion methods on {@code TypeConvertingMap} and + * the servlet attribute extensions, keeping the conversion logic in a single place. + * + * <p>This is an internal class and is not part of the public Grails API. + * + * @since 7.2 + */ +@SuppressWarnings({ "rawtypes", "unchecked" }) +public final class TypeConverters { Review Comment: I'm assuming this logic was a straight copy from the original methods? ########## grails-data-graphql/core/src/main/groovy/org/grails/gorm/graphql/fetcher/impl/DeleteEntityDataFetcher.groovy: ########## @@ -48,7 +48,7 @@ class DeleteEntityDataFetcher<T> extends DefaultGormDataFetcher<T> implements De } protected void deleteInstance(GormEntity instance) { - instance.delete(failOnError: true) Review Comment: We should set the merge base to https://github.com/apache/grails-core/pull/15716 to remove this. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
