YannickB / odoo-hosting

Other
64 stars 50 forks source link

Travis Issues #134

Open YannickB opened 7 years ago

YannickB commented 7 years ago

Let's continue the discussion about travis here

So after my refactor today, travis is green! https://travis-ci.org/clouder-community/clouder

But even if it's green, there is some warnings : https://travis-ci.org/clouder-community/clouder/jobs/167125877 -W7930 : Theses files should not be included in the manifest, these are template files which will be sent to the container after deployment, there are not meant to be used by Odoo. -C8101 : Well this is not an OCA project (at least yet), we can't use their name like that. -W7907 : The hell is wrong with Travis here, theses fields are in one2many! -E8102 : We force cr.commit() before each system command. Before issuing a command on server, we want to be sure Odoo will now revert back and for example remove a container record while it was created on server but an Odoo error happened later. -W7903 : @nicolas-petit can you check this one ? -R8110 : @nicolas-petit I think you let this function in old api because you had no other choice, but we'll need to fix it, not gonna work in v10 anyway -R7980 : I don't want to merge theses functions, I want to keep theses functions in different files for code readibility

@lasley your thoughts? Is there any way to disable some of the checks ?

lasley commented 7 years ago

Hooray 🍏 ! The other failing Lints are in beta, so could be erroneous (like W7907). We're good to ignore them for the most part.

E8102 - Agreed on the need for the commit. Connector does this as well. It would be best to centralize to a commit method & add a check to not commit when testing such as in connector

IMO when we identify that the check is valid but should be ignored, such as E8102, it can be disabled:

cr.commit()  #pylint: disable=E8102

More examples of disables - https://pylint.readthedocs.io/en/latest/user_guide/message-control.html

lasley commented 7 years ago

This is good to close I think