why not just to add an entry in the manifest to say which model has been inherited/modified as it AFAIU the main reason raised.
splitting the modification in files by model make sense for xml but for py file as far as they contain a bug number of lines of code we won't gain any benefit. my 2 cents... 2014-10-20 16:57 GMT+02:00 Pedro Manuel Baeza Romero <pedro.ba...@gmail.com> : > Sandy, for me it's OK this way. > > Regards. > > 2014-10-20 16:52 GMT+02:00 Sandy Carter <sandy.car...@savoirfairelinux.com > >: > >> Hi, >> >> My issue with leaving it to subjective criticism is: >> >> Firstly, if you're new and learning the ropes to contribution to OCA and >> you do your homework, you're going to do two things to make sure your >> module gets integrated. First you'll check existing modules and emulate >> what they do and second you'll read the guidelines. Like I mentioned at >> the beginning, there are many cases of X models in files named like >> 'product.py', and if you look at the guidelines[1], it is still bare in >> many parts. >> >> We see this today, a new dev will do her work, prepare her module using >> these guidelines. When he finally does a pull request, we come down on >> him with these subjective reviews, that we are used to, but to the dev, >> these seem completely arbitrary. The result is often that the new dev >> will redo her module multiple times with no more guidelines than the >> review of a select few contributors who have had the time to review her >> code. Another possible outcome, is that these PRs get stuck in limbo >> where the dev might have moved on to another project. >> >> A second problem with subjective criticism is when a disagreement is met >> or a dev making a PR is unwilling to change his code and chooses to >> argue every point. >> >> Without the points in the review page, a subjective review has very >> little weight, you can't just point at the page and say: "These are the >> standards we do our review by, this is what everyone who contributes >> agrees to for the sake of uniform coding style". Instead we find >> ourselves saying: "This is better because it is", "This is what is done >> in most modules", "This is not yet on the page". >> >> These two situations happen quite often. I have multiple less >> experienced Odoo developers come to me frustrated because their PRs are >> being stalled for what seems to them to be arbitrary reasons. The first >> thing they ask is usually "What does she mean by X? This seems >> unreasonable." I often reply with "That kind of sucks but I agree with >> his review". the second thing they ask is "Where can I find all these >> guidelines so I can avoid being stalled on getting my code merged?" And >> for that, I would love to be able to refer to the guidelines page[1]. At >> the present, I can't really... >> >> What the consensus I seem to get out of this is: Often it is better to >> separate the models (especially in xml), but there are some acceptable >> exceptions: >> >> * Small changes to models >> * One to many relationships (single column add to related model) >> * At some point, a small change becomes to big and the file must be >> split. >> * All changes are very closely related to a same feature >> * Wizards >> * Connectors (I'm adding this one) >> >> No one seemed to object to naming the Class, .py file and xml files >> according to the model (or largest model in the case of filenames). >> >> Is this something we can agree on? >> >> [1] http://odoo-community.org/page/website.how-to >> >> Le 2014-10-20 05:39, Leonardo Pistone a écrit : >> > Hi all, >> > >> > I understand the convenience of finding classes easily, and the >> > reasons behind this are sound. Still, I do not sopport such a rule on >> > splitting files. >> > >> > Still, I think we can and should work to improve subjective quality >> > and clarity of code. Example: after the machine checked lint, I think >> > we can (and should) gradually move to comments like >> > >> > - this method / this file is long, can you split it? >> > - I don't understand what this is for, can you comment or refactor? >> > - why do we need that? (comment or refactor) >> > - (and one day, gradually...) could you decouple this logic from the >> > database schema to a pure class? >> > >> > TL;DR I oppose a rule on file length and such, but I favor subjective >> > criticism to get the code as clear as possible. >> > >> > Thanks to all for the involvement! >> > >> > On 10/19/2014 10:46 PM, Graeme Gellatly wrote: >> > But for me this comes >> >> down to personal preference, not sure it needs to be a convention. >> > >> > _______________________________________________ >> > Mailing list: https://launchpad.net/~openerp-community >> > Post to : openerp-community@lists.launchpad.net >> > Unsubscribe : https://launchpad.net/~openerp-community >> > More help : https://help.launchpad.net/ListHelp >> > >> >> >> _______________________________________________ >> Mailing list: https://launchpad.net/~openerp-community >> Post to : openerp-community@lists.launchpad.net >> Unsubscribe : https://launchpad.net/~openerp-community >> More help : https://help.launchpad.net/ListHelp >> >> > > _______________________________________________ > Mailing list: https://launchpad.net/~openerp-community > Post to : openerp-community@lists.launchpad.net > Unsubscribe : https://launchpad.net/~openerp-community > More help : https://help.launchpad.net/ListHelp > >
_______________________________________________ Mailing list: https://launchpad.net/~openerp-community Post to : openerp-community@lists.launchpad.net Unsubscribe : https://launchpad.net/~openerp-community More help : https://help.launchpad.net/ListHelp
