On 18/03/2010, at 10:50 AM, Adam Heath wrote: > Jacopo Cappellato wrote: >> Adam, >> >> this is going to be a long thread and I don't want it to be. >> I already tried to explain why I decided to change the primary key of >> OrderShipment without following the deprecation mechanism (that I think it >> is a very good best practice) in this specific situation. >> I perfectly know when and how the shipment plans (i.e. the feature for which >> the OrderShipment entity was introduced) were implemented: I know because I >> contributed it (and the OrderShipment entity of course). >> I also know that it was only partially implemented and so I would be very >> surprised (especially because no one over the years ever asked or reported >> bugs etc.. about it - apart from me that was aware of its existence) if it >> is really used in production by someone. This is simply to explain that I >> was not irresponsible when I did it. >> >> However you are right if you say that I can't be completely sure if someone >> somewhere is using the entity. Also, lately the community seems to be more >> inclined to transform "best practices" (to be applied "cum granu salis") >> into "policies" to be applied blindly. >> In some way this is good because it requires less skills and creativity (but >> more pragmatism) and also less skilled persons can become good reviewers >> just by following written rules. >> This is probably less interesting but maybe necessary given the grow of the >> community. >> >> So ok, I am convinced now, we have to deprecate that entity etc. >> >> What does your book of policies says about this: is it something I *have* to >> do as a punishment for this bad commit or will you take care of it? > > If I implied that it had to be you to do it, I apoligize. I didn't > mean it to sound that way. > > I was trying to discuss with you about the reasons behind this change, > so that we could all agree as to what the reasons and use cases were. > > Now that it seems we are on the same page, we can go about fixing this > as a team. I'll admit, however, that I have never gone thru an entity > deprecation exercise. And OrderShipment is a very low-level entity, > that has tons of references, so doing this right could be very > complex. The reason why I was so heated was that I understood this > would be a lot of work, and I was trying to see if there is a better > way than changing the primary key. > > The reason the original committer is generally the best one to fix > their original commit, is because that original committer knows the > most about what they were trying to do. That original person knows > their own code they wrote, and probably has an understand of the rest > of the files that they were modifying. > > However, due to the scope of this deprecation, we need the rest of the > community to help out. Unless we decide to revert this primary key > change, and due this a different way. > >> PS: BTW, if no one will implement the deprecation pattern in the next few >> days I will do that, no problem. > > There's no big hurry. My previously mentioned importer already had > the shipGroupSeqId available(it was hard-coded to 00001, as it's > somewhat simple), so it wasn't a big change for me personally.
I'm yet to form an opinion on whether the approach Jacopo took was a valid one, I've been looking at the code and I don't yet understand what problems this change may have caused for downstream users. Adam, would you mind explaining in more detail how the change has effected you negatively? Thanks Scott
smime.p7s
Description: S/MIME cryptographic signature
