grails-data-mapping icon indicating copy to clipboard operation
grails-data-mapping copied to clipboard

Published 20 hours ago • verified publisher icon grails

PersistentProperty, EntityAccess, EntityReflector formalities

Open namgang opened this issue 6 years ago • 5 comments

Formal issues in key interfaces.

  • Interface org.grails.datastore.mapping.model.PersistentProperty has a misspelled method name. getCapitilizedName should be deprecated, clone to getCapitalizedName (corrected spelling).
  • Interface org.grails.datastore.mapping.engine.EntityAccess has a superfluous method. Deprecate either of getPropertyValue or getProperty.
  • org.grails.datastore.mapping.reflect.EntityReflector has a misspelled method name. getPersitentEntity should be deprecated, clone to getPersistentEntity (corrected spelling).

namgang avatar Aug 12 '18 17:08 namgang

Even if there are some methods misspelled this is a breaking change because we're changing public api. I propose to move this to GORM 7.0 milestone. What do you think @puneetbehl?

ilopmar avatar Oct 10 '18 07:10 ilopmar

I am not sure @ilopmar. As we are talking about deprecating them. And, I do not see any harm in doing so as the methods will still be there.

puneetbehl avatar Oct 10 '18 09:10 puneetbehl

You are right, it is about deprecating and not removing method. In any case I don't think we should do the change in a 6.1.x release. I still think it is better to move it to 7.0.0

ilopmar avatar Oct 10 '18 10:10 ilopmar

@ilopmar No harm in deprecating a method in 6.1.x if the change is simply picking a method with a slightly different name. If we waited to deprecate until 7, we would have to wait until 8 to remove them

jameskleeh avatar Oct 10 '18 13:10 jameskleeh

Ok, fair enough. Then we deprecate now so we can remove later in 7. Thanks for the clarification :+1:

ilopmar avatar Oct 10 '18 13:10 ilopmar