Re: [openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.
On 21/12, Walter A. Boring IV wrote: > On 12/21/2015 06:40 AM, Philipp Marek wrote: > >Hi everybody, > > > >in the current patch https://review.openstack.org/#/c/259973/1 the test > >script needs to use a lot of the constant definitions of the backend driver > >it's using (DRBDmanage). > > > >As the DRBDmanage libraries need not be installed on the CI nodes, I'm > >providing a minimum of upstream files, accumulated in a separate directory > >- they get imported and "fixed" to the expected location, so that the > >driver that should be tested runs as if DRBDmanage is installed. > > > > > >My problem is now that the upstream project doesn't accept all the pep8 > >conventions like Openstack does; so the CI run > > > > http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html > >gives a lot of messages like "E221 multiple spaces before operator" and > >similar. (It even crashes during AST parsing ;) > > > > > >So, I can see these options now: > > > > * Make pep8 ignore these files - they're only used by one test script, > > and are never used in production anyway. > > + Simple > > + New upstream files can simply be dropped in as needed > > - bad example? > > * Reformat the files to conform to pep8 > > - some work for every new version that needs to be incorporated > > - can't be compared for equality with upstream any more > > - might result in mismatches later on, ie. production code uses > > different values from test code > I would suggest this option. We don't want to allow code in cinder that > bypasses the pep8 checks. Since you are trying to get drbd support into > Cinder, it then falls upon you to make sure the code you are submitting > follows the same standards as the rest of the project. > > Walt > I would normally suggest option #4, but considering listed downsides this looks like the best alternative, as Walter suggests. To reduce the work for every release you could probably automate it using autopep8. Cheers, Gorka. > > > > * Throw upstream files away, and do "manual" fakes > > - A lot of work > > - Work needed for every new needed constant > > - lots of duplicated code > > - might result in mismatches later on, ie. production code uses > > different values from test code > > + whole checkout still "clean" for pep8 > > > > * Require DRBDmanage to be installed > > + uses same values as upstream and production > > - Need to get it upstream into PyPi > > - Meaning delay > > - delay for every new release of DRBDmanage > > - Might not even be compatible with every used distribution/CI > > out there > > > > > >I would prefer the first option - make pep8 ignore these files. > >But I'm only a small player here, what's the opinion of the Cinder cores? > >Would that be acceptable? > > > > > >Regards, > > > >Phil > > > >__ > >OpenStack Development Mailing List (not for usage questions) > >Unsubscribe: openstack-dev-requ...@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-requ...@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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.
On 12/21/2015 06:40 AM, Philipp Marek wrote: Hi everybody, in the current patch https://review.openstack.org/#/c/259973/1 the test script needs to use a lot of the constant definitions of the backend driver it's using (DRBDmanage). As the DRBDmanage libraries need not be installed on the CI nodes, I'm providing a minimum of upstream files, accumulated in a separate directory - they get imported and "fixed" to the expected location, so that the driver that should be tested runs as if DRBDmanage is installed. My problem is now that the upstream project doesn't accept all the pep8 conventions like Openstack does; so the CI run http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html gives a lot of messages like "E221 multiple spaces before operator" and similar. (It even crashes during AST parsing ;) So, I can see these options now: * Make pep8 ignore these files - they're only used by one test script, and are never used in production anyway. + Simple + New upstream files can simply be dropped in as needed - bad example? * Reformat the files to conform to pep8 - some work for every new version that needs to be incorporated - can't be compared for equality with upstream any more - might result in mismatches later on, ie. production code uses different values from test code I would suggest this option. We don't want to allow code in cinder that bypasses the pep8 checks. Since you are trying to get drbd support into Cinder, it then falls upon you to make sure the code you are submitting follows the same standards as the rest of the project. Walt * Throw upstream files away, and do "manual" fakes - A lot of work - Work needed for every new needed constant - lots of duplicated code - might result in mismatches later on, ie. production code uses different values from test code + whole checkout still "clean" for pep8 * Require DRBDmanage to be installed + uses same values as upstream and production - Need to get it upstream into PyPi - Meaning delay - delay for every new release of DRBDmanage - Might not even be compatible with every used distribution/CI out there I would prefer the first option - make pep8 ignore these files. But I'm only a small player here, what's the opinion of the Cinder cores? Would that be acceptable? Regards, Phil __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@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-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [Cinder][DRBD] questions about pep8/flake8 etc.
Hi everybody, in the current patch https://review.openstack.org/#/c/259973/1 the test script needs to use a lot of the constant definitions of the backend driver it's using (DRBDmanage). As the DRBDmanage libraries need not be installed on the CI nodes, I'm providing a minimum of upstream files, accumulated in a separate directory - they get imported and "fixed" to the expected location, so that the driver that should be tested runs as if DRBDmanage is installed. My problem is now that the upstream project doesn't accept all the pep8 conventions like Openstack does; so the CI run http://logs.openstack.org/73/259973/1/check/gate-cinder-pep8/5032b16/console.html gives a lot of messages like "E221 multiple spaces before operator" and similar. (It even crashes during AST parsing ;) So, I can see these options now: * Make pep8 ignore these files - they're only used by one test script, and are never used in production anyway. + Simple + New upstream files can simply be dropped in as needed - bad example? * Reformat the files to conform to pep8 - some work for every new version that needs to be incorporated - can't be compared for equality with upstream any more - might result in mismatches later on, ie. production code uses different values from test code * Throw upstream files away, and do "manual" fakes - A lot of work - Work needed for every new needed constant - lots of duplicated code - might result in mismatches later on, ie. production code uses different values from test code + whole checkout still "clean" for pep8 * Require DRBDmanage to be installed + uses same values as upstream and production - Need to get it upstream into PyPi - Meaning delay - delay for every new release of DRBDmanage - Might not even be compatible with every used distribution/CI out there I would prefer the first option - make pep8 ignore these files. But I'm only a small player here, what's the opinion of the Cinder cores? Would that be acceptable? Regards, Phil __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev