[openstack-dev] [all] Why do we have project specific hacking rules?

Andrew Laski andrew at lascii.com
Sun Oct 2 19:55:04 UTC 2016



On Sun, Oct 2, 2016, at 11:02 AM, Matt Riedemann wrote:
> On 10/2/2016 5:47 AM, Amrith Kumar wrote:
> > It was my understanding that hacking rules were like the 'Ten Commandments', the 'Four Opens'; things that were universally true across all projects and an attempt to bring standardization to all OpenStack code.
> >
> > How come we then have extensive project specific hacking rules? Why not make these "nova-specific" rules OpenStack wide?
> >
> > I looked at the checks.py file that Matt linked to below, and I can't see anything really "nova-specific"; i.e. all of them would translate just fine to Trove. Is there some reason they can't become OpenStack wide rules?

For some of them there are good reasons not to make them OpenStack wide
checks.

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n155
no db usage in virt driver.

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n168
no db session in public db api

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n201
virt drivers aren't importing modules from other virt drivers

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n220
virt drivers aren't using config options from other virt drivers

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n406
api microversion decorator is first decorator when used

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n616
check that nova specific utility method is used rather than
greenthread|eventlet.spawn|spawn_n

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n643
config options are registered in a central place

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n667
policy options are registered in a central place

http://git.openstack.org/cgit/openstack/nova/tree/nova/hacking/checks.py?h=stable/newton#n681
a nova specific context.can() method must be used for policy enforcement
rather than policy.enforce|authorize

All of these checks are either very specific to Nova or are
philosophical choices that have been made in Nova but don't need to be
shared by other projects.

And as Matt points out below some of the checks not listed above depend
on specific paths being used in the check which makes it more difficult
to share those checks.

A further hindrance to moving these checks to be OpenStack wide is that
each check violation is a failure. So in order to roll a new check out
across projects it requires a lot of churn in each project which
violates the checks. I would prefer to leave it up to each project to
make the decision to incorporate a new check and take the review load.
Ultimately what I think sounds good would be a central listing of checks
that are in use across projects and then leave it up to each project to
replicate those that they like.


> >
> > -amrith
> >
> >> -----Original Message-----
> >> From: Ihar Hrachyshka [mailto:ihrachys at redhat.com]
> >> Sent: Sunday, October 02, 2016 5:25 AM
> >> To: OpenStack Development Mailing List (not for usage questions)
> >> <openstack-dev at lists.openstack.org>
> >> Subject: Re: [openstack-dev] [all][i18n] do we need translation mark for
> >> strings in tests?
> >>
> >> Matt Riedemann <mriedem at linux.vnet.ibm.com> wrote:
> >>
> >>> On 10/1/2016 5:49 PM, Matt Riedemann wrote:
> >>>> No you shouldn't need to mark strings for translation in test code. I
> >>>> believe we have a hacking rule for marking LOG.info/warning/error
> >>>> messages for translation but it should skip test directories.
> >>>
> >>> Ah I guess the hacking rule is nova-specific:
> >>>
> >>>
> >> https://github.com/openstack/nova/blob/2851ceaed3010c19f42e308be637b952eda
> >> b092a/nova/hacking/checks.py#L342
> >>
> >> We have a similar one in neutron; but note that it does not explicitly
> >> FAIL
> >> on translation markers in test code; it instead fails on no markers in
> >> production code, which is different different.
> >>
> >> Ihar
> >>
> >> __________________________________________________________________________
> >> OpenStack Development Mailing List (not for usage questions)
> >> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> > __________________________________________________________________________
> > OpenStack Development Mailing List (not for usage questions)
> > Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> >
> 
> Well for one thing the specific one I pointed out has hard-coded nova 
> paths in it which might not work for other projects.
> 
> -- 
> 
> Thanks,
> 
> Matt Riedemann
> 
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe:
> OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev



More information about the OpenStack-dev mailing list