summaryrefslogtreecommitdiff
path: root/HACKING.rst
Commit message (Collapse)AuthorAgeFilesLines
* Performance: leverage dict comprehension in PEP-0274ChangBo Guo(gcb)2015-01-161-0/+1
| | | | | | | | | | | | | | | | | | | | | PEP-0274 introduced dict comprehensions to replace dict constructor with a sequence of length-2 sequences, these are benefits copied from [1]: The dictionary constructor approach has two distinct disadvantages from the proposed syntax though. First, it isn't as legible as a dict comprehension. Second, it forces the programmer to create an in-core list object first, which could be expensive. Nova dropped python 2.6 support, we can leverage this now. There is deep dive about PEP-0274[2] and basic tests about performance[3]. Note: This commit doesn't handle dict constructor with kwagrs. This commit also adds a hacking rule. [1]http://legacy.python.org/dev/peps/pep-0274/ [2]http://doughellmann.com/2012/11/12/the-performance-impact-of-using-dict-instead-of-in-cpython-2-7-2.html [3]http://paste.openstack.org/show/154798/ Change-Id: Ifb5cb05b9cc2b8758d5a8e34f7792470a73d7c40
* Do not use deprecated assertRaisesRegexp()Davanum Srinivas2015-01-141-0/+1
| | | | | | | | | | | | The unit test log ends up with DeprecationWarning(s) from the outdated calls to assertRaisesRegexp. We should switch to using assertRaisesRegex instead. This commit eliminates these warnings from the log and the hacking rule N344 ensures that folks don't end up adding fresh code down the line with the outdated assertRaisesRegexp as well Partial-Bug: #1407736 Change-Id: Ifba672f7568d5159c63bf88c534812e4e3a26d5a
* Merge "Added hacking rule for assertTrue/False(A in B)"Jenkins2015-01-131-0/+2
|\
| * Added hacking rule for assertTrue/False(A in B)Sergey Nikitin2015-01-071-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following replacements were done in tests to have clearer messages in case of failure: - assertTrue(a in b) with assertIn(a, b) - assertTrue(a not in b) with assertNotIn(a, b) - assertFalse(a in b) with assertNotIn(a, b) The error message would now be like: 'abc' not in ['a', 'b', 'c'] rather than: 'False is not True'. Change-Id: I92d039731f17b731d302c35fb619300078b8a638
* | Replace Hacking N315 with H105Joe Gordon2015-01-101-2/+0
|/ | | | | | | Don't use author tags is H105 in hacking 0.10, so drop local rule N315 in favor of H105. Change-Id: I38bf2ff5247817d58723f28e87607f16f3d9c374
* Prevent new code from using namespaced oslo importsDavanum Srinivas2014-12-281-0/+1
| | | | | | | | | | Ib63da2d845843410634a1df0261af33b973daf32 is an example where new code had to be fixed for oslo_concurrency imports. Let us add a hacking check to prevent such regression. When we update to new oslo libraries with non-namespace'd imports, we should update the regexp in this hacking rule Change-Id: I44536d477d06ddc1205b824bcb888b666405dce3
* Adds hacking check for api_version decoratorMatthew Gilliard2014-12-081-0/+1
| | | | | | | | | | | | | | | | | | | | | | Because of the implementation of this decorator and the controller's metaclass, the api_version decorator must be the outermost (ie first) decorator wherever it is used. This patch adds a hacking check to ensure that this is the case. This decorator is intended to be used on multiple methods with the same name which offends F811, so '#noqa' is needed too. This will be fixed in a separate patch. Bad: @some_decorator # noqa <-- '# noqa' to avoid F811 @api_version("2.5") <-- Error, needs to be the first decorator def my_api_method... Good: @api_version("2.5") # noqa <-- this line still needs '# noqa' for F811 @some_decorator def my_api_method... Change-Id: I579c0061f03d788c477c5424d4d00ec7a6e721e1
* Replacement `_` on `_LW` in all LOG.warning part 1Mike Durnosvistov2014-11-201-0/+2
| | | | | | | | | | | | | | | | | | oslo.i18n uses different marker functions to separate the translatable messages into different catalogs, which the translation teams can prioritize translating. For details, please refer to: http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack There were not marker fuctions some places in directory network. This commit makes changes: * Add missing marker functions * Use ',' instead of '%' while adding variables to log messages Added a hacking rule for the warning about checking translation for it and checking logging level `warning` instead alias `warn`. Change-Id: I2bced49dc5a0408a94d5d20d85b20c682886edbe
* Replacement `_` on `_LE` in all LOG.exceptionMike Durnosvistov2014-11-201-0/+1
| | | | | | | | | | | | | | | | | oslo.i18n uses different marker functions to separate the translatable messages into different catalogs, which the translation teams can prioritize translating. For details, please refer to: http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack There were not marker fuctions some places in directory network. This commit makes changes: * Add missing marker functions * Use ',' instead of '%' while adding variables to log messages Added a hacking rule for the log exception about checking translation for it. Change-Id: If80ea6f177bb65afcdffce71550bb38fedcc54eb
* Replacement `_` on `_LI` in all LOG.info - part 1Mike Durnosvistov2014-11-201-0/+1
| | | | | | | | | | | | | | | | | oslo.i18n uses different marker functions to separate the translatable messages into different catalogs, which the translation teams can prioritize translating. For details, please refer to: http://docs.openstack.org/developer/oslo.i18n/guidelines.html#guidelines-for-use-in-openstack There were not marker fuctions some places in directory network. This commit makes changes: * Add missing marker functions * Use ',' instead of '%' while adding variables to log messages Added a hacking rule for the log info about checking translation for it. Change-Id: I96766d723b01082339876ed94bbaa77783322b8c
* Remove use of unicode on exceptionsJames Carey2014-10-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | Casting exceptions to unicode using unicode() can interfere with the proper translation of the exception message. This is especially true when lazy translation is enabled, since it causes the exception message to be translated immediately using the default locale. This is the same problem caused by using str on exceptions, which was fixed by https://review.openstack.org/#/c/116054/ In addition to fixing these cases, this patch updates hacking check N325, which checks for the use of str() on exceptions, to also check for the use of unicode(). In order to make it clear which cases that cannot be caught by the hacking check have been inspected, they have been converted to using six.text_type() instead of unicode(). Closes-bug: #1380806 Change-Id: I87bb94fa76458e028beba28d092320057b53f70a
* mock.assert_called_once() is not a valid methodDavanum Srinivas2014-09-161-0/+1
| | | | | | | | | | | | | | mock.assert_called_once() is a no-op that tests nothing. Instead with mock.assert_called_once_with() should be used (or use assertEqual(1, mock_obj.call_count) if you don't want to check parameters). Add a new HACKING rule for nova to prevent assert_called_once() usage from creeping in. Closes-Bug: #1365751 Change-Id: I1055093a1c31792b6411a3cd46c80b8bcaaf78c1
* Merge "Move to oslo.db"Jenkins2014-09-071-1/+1
|\
| * Move to oslo.dbAndrey Kurilin2014-09-051-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace common oslo code nova.openstack.common.db by usage of oslo.db library and remove common code. Replaced catching of raw sqlalchemy exceptions by catches of oslo.db exceptions(such as DBError, DBDuplicateEntry, etc). Co-Authored-By: Eugeniya Kudryashova <ekudryashova@mirantis.com> Closes-Bug: #1364986 Closes-Bug: #1353131 Closes-Bug: #1283987 Closes-Bug: #1274523 Change-Id: I0649539e071b2318ec85ed5d70259c949408e64b
* | Remove concatenation with translated messagesJames Carey2014-08-271-0/+1
|/ | | | | | | | | | | | | | | | | Translated messages should not be expanded by concatenation. Instead the additional text should be replacement text or part of the translated message to allow the translators as much information as possible when doing the translations. Also, when lazy translation is enabled, the object returned does not support concatenation and raises an exception. Thus this patch is needed to support blueprint: i18n-enablement. This patch includes a hacking check for concatenation with a translated message. Change-Id: I8b368d275faa14b4750735445321874ce1da37d5 Partially-Implements: blueprint i18n-enablement
* Remove use of str on exceptionsJames Carey2014-08-271-0/+1
| | | | | | | | | | | | | | | | | | | | | | An exception's message can be a translatable message. If it is, the message can contain unicode characters which will cause str to fail. In cases where the message is explicity needed, the use of str is replaced with the use of six.text_type. When an exception is used as a replacement string in a format string, the logger correctly handles it without the need for str, so it is removed. In addition to the case where a translatable message contains unicode, enabling lazy translation in the oslo.i18n library causes translatable messages to be replaced with an object that does not support str, this causes all calls to str on a translatable message to fail. Thus this patch is also needed to support blueprint: i18n-enablement. This patch includes a hacking check for use of str() on exceptions identified in except statements. Change-Id: Idb426d7f710ea69b835f70d0e883e93e9b9111d2 Partially-Implements: blueprint i18n-enablement
* Hacking: a new hacking check was added that used an existing numberGary Kotton2014-08-141-0/+1
| | | | | | | | | | Commit 243879f5c51fc45f03491bcb78765945ddf76be8 added in a new hacking check that used an existing number. The new number is 324 (and not 323) Change-Id: I7e604a408387438105c435ad16a1fa3d6491b642 Closes-bug: #1356815
* Add hacking check for explicit import of _()Jay S. Bryant2014-08-031-0/+1
| | | | | | | | | | | | | | To ensure the right message catalog is used when translating messages we need to make sure to explicitly import '_' in any files that use that function. We cannot count on unit test to catch cases where the user has forgotten to import the _() function. This hacking check ensures that the function has been imported anywhere that it is used. Unit tests for the hacking check are included. Change-Id: I9d8101916bcb449345d3123617c2ac75776d053e
* Removes the use of mutables as default argsChangBo Guo(gcb)2014-06-181-0/+1
| | | | | | | | | | | Passing mutable objects as default args is a known Python pitfall. We'd better avoid this. This commit changes mutable default args with None, then use 'arg = arg or {}', 'arg = arg or []'. For unit code which doesn't use the args , just set with None. This commit also adds hacking check. Closes-Bug: #1327473 Change-Id: I5a8492bf8ffef8e000b13b6bdfaef1968b96f816
* Add missing translation supportGary Kotton2014-06-021-0/+1
| | | | | | | | | | | | | | | Update a number of files to add missing translation support. The patch adds a new hacking check - N321. This ensures that all log messages, except debug ones, have translations. A '# noqa' indicates that the validation will not be done on the specific log message. This should be used in cases where the translations do not need to be done, for example, the log message is logging raw data. Closes-bug: #1290261 Change-Id: Ib2ca0bfaaf432e15448c96619682c2cfd073fbbc
* Update HACKING.rst to include N320Gary Kotton2014-06-021-0/+2
| | | | | | | | Commit 9235ada8c2c250603dc5b299cc08bb7a982d4fc6 did not add in the updated hacking rule. Change-Id: If2daf6d2e7008c36d0aba6270ac034522dcb2e2b Closes-bug: #1325812
* Add a reference to the nova developer documentationMichael H Wilson2014-05-191-0/+4
| | | | | | | | I've showed these docs to more people than I care to. It would be extremely helpful to reference them in HACKING, which is generally the first thing people read if they intend to hack. Change-Id: I9194d98f5525e29711b4a1b26414a50b8ceba525
* Hacking: add rule number to HACKING.rstGary Kotton2014-04-271-0/+1
| | | | | | | | Commit ac8bce63f8a7ec8a2ebb214ea7f86ee4f8adecae added hacking rules N319. This was not documented in the file HACKING.rst. Change-Id: Ibf7917aaada88db5afe1387859a387481ec05118 Closes-bug: #1313322
* Replace assertEqual(None, *) with assertIsNone in testszhang-jinnan2014-02-161-0/+2
| | | | | | | Replace assertEqual(None, *) with assertIsNone in tests to have more clear messages in case of failure. Change-Id: I0d38a82e78fbe94657ab9a71c08422007843d179
* Change assertTrue(isinstance()) by optimal assertMarcos Lobo2014-02-121-0/+4
| | | | | | | | | Some of tests use different method of assertTrue(isinstance(A, B)) or assertEqual(type(A), B). The correct way is to use assertIsInstance(A, B) provided by testtools. Change-Id: I4a5413f9d90d2e581044885a440a46bf3d76598f Closes-Bug: #1268480
* Remove @author from copyright statements.Michael Still2014-02-121-0/+2
| | | | | | | | | We have git to track authorship, so let's not pad source files with it as well. Co-authored-by: Joe Gordon <joe.gordon0@gmail.com> Change-Id: Ic2d62d6743f7716c086749cd99922b6c496771d4
* Renumber some nova hacking checksDaniel P. Berrange2014-02-101-1/+3
| | | | | | | | | | | Most of the nova hacking checks had picked numbers starting from approx N300 onwards. Two recent additions randomly picked N123 and N500. Renumber these to N313 and N314 and make sure they're documented in HACKING.rst. Mention the guidelines in the hacking source file for benefit of future authors Change-Id: Ia3eb4cb9a4ac7409db7eba9e1689f4a5780b8795
* Add hacking test to block cross-virt driver code usageDaniel P. Berrange2014-02-041-0/+6
| | | | | | | | | | | | | | | | The baremetal driver has been accessing some libvirt driver private classes and config variables. Any code that is intended to be shared across virt drivers should be in a common package instead. Add a hacking file to enforce this for imports and config options so that no further problems like this are made in the future. NB, this skips verification of the baremetal driver until bugs 1261826 and 1261827 are fixed, or the baremetal driver is removed, whichever comes first. Change-Id: Ifbfe597795795a847830f9bd937dc459cd37d118 Closes-Bug: #1261842
* Remove vi modelinesliu-sheng2014-02-031-0/+1
| | | | | | | | | | We don't need to have the vi modelines in each source file, it can be set in a user's vimrc if required. Also a check is added to hacking to detect if they are re-added. Change-Id: I347307a5145b2760c69085b6ca850d6a9137ffc6 Closes-Bug: #1229324
* Updates OpenStack Style Commandments linkKeshava Bharadwaj2013-10-161-1/+1
| | | | | | | The current link in the HACKING file is broken. This references the correct location for contributors to view. Change-Id: I3cd4841035400d09a3e3e3369bb3a2a8c4bdee30
* Use timeutils.utcnow() throughout the codeRoman Podolyaka2013-07-191-0/+2
| | | | | | | | | | | | timeutils.utcnow() should be used instead of direct calls to datetime.datetime.utcnow() to make it easy to override its return value in tests. Add a hacking rule to prevent regressions. Fixes bug 1200141. Change-Id: I170dbd7c9093bd627e2a0d5984b7ad1bf105c8d5
* Add HACKING check for db session paramDevananda van der Veen2013-07-021-1/+2
| | | | | | | | | | Add a HACKING check to enforce that public db/api and db/sqlalchemy/api methods to not accept a 'session' parameter. This check is initially disabled, since it is failing ~24 times right now, but will be enabled once bp/db-session-cleanup is complete. Change-Id: Ib89eea58555032dd142d4e21e62d66e2726f0d06
* Clean up and make HACKING.rst DRYerJoe Gordon2013-07-011-290/+16
| | | | | | | Reference the OpenStack hacking guide in HACKING.rst and remove duplicated entries. Add new section for nova specific rules. Change-Id: I704a49a0675acf1870953c76eba4978a1c4490eb
* Merge "Add notes about how doc generation works."Jenkins2013-06-131-0/+21
|\
| * Add notes about how doc generation works.Monty Taylor2013-06-021-0/+21
| | | | | | | | Change-Id: I562c743798aeae5e49bd2f96944c3cacd776a53d
* | Replace openstack-common with oslo in HACKING.rstThomas Bechtold2013-06-071-6/+6
| | | | | | | | Change-Id: I8c6426983ec374e903c64108b990e5d4550e3fcc
* | Improve Python 3.x compatibilityDirk Mueller2013-06-011-0/+19
|/ | | | | | | | | Mechanical translation of the deprecated except x,y: construct with except x as y: The latter works with any Python >= 2.6. Add Hacking check. Change-Id: I845829d97d379c1cd9b3a77e7e5786586f263b64
* docs should indicate proper git commit limitSteven Dake2013-03-041-2/+2
| | | | | | | | | | | The nova gate should recommend 50 characters or less for a git commit but actually enforce 72 characters. This patch changes the hacking.rst docs to indicate the actual limit is 72 characters rather then 50 characters. Change-Id: I47f1f1f1007f5744bf1fef419df7e033803b4a53 Fixes: Bug #1144840
* Fix broken logging imports.Vishvananda Ishaya2013-02-191-1/+0
| | | | | | | | This fixes all of the files incorrectly importing logging directly and removes the workaround in hacking.py that was due to improper from nova.openstack.common.log import logging statements. Change-Id: Icfc25dc148c4a7b5fa7f6a7b609cd6c3d94efee1
* Module import style checking changesAttila Fazekas2013-02-131-0/+3
| | | | | | | | | | | | | | * Implementing the * import detection (it is disabled for now) * New style relative import testing based on syntax rules * Old style relative import testing based on module search * Inspection based solution replaced by PYTHONPATH search in order to avoid module compile and initialization steps (code execution) in a syntax checking phase. This solution is faster and safer, but does not able to recognize modules added dynamically to the module scope. Change-Id: Ifc871f4fdbcd4a9a736170ceb4475f4f2cbe66bc
* Update docs about testing.Monty Taylor2013-02-111-1/+21
| | | | | | | | Add an entry to the HACKING file about testr. While in there, noticed a reference to the now-defunct nova/testing dir. Fixed that, moved the testing README into nova/tests and remove the nova/testing dir. Change-Id: Ibf6fb82658ba73eee9123fa53b340d0b72afb292
* Update HACKING.rst per recent changesZhongyue Luo2013-02-041-3/+10
| | | | | | | | | | | | Added "is not" usage with examples * https://review.openstack.org/#/c/20865/ Fixed "not in" usage description * https://review.openstack.org/#/c/20875/ Fixed some "not X in Y" corner cases. Change-Id: I7534ef73e6fd525fd8f4bee594a4b37524699c08
* Fixes 'not in' operator usageZhongyue Luo2013-01-311-0/+12
| | | | Change-Id: I1e26a8fcb9fa564308e63c11a72aaa55119e4eee
* don't allow crs in the codeSean Dague2013-01-161-0/+1
| | | | | | | | triggered by this slipping into quantum, now that we have more windows developers we should ensure that \r\n doesn't land in the code as a line ending. This check prevents it. Change-Id: I0a82be0e74915d3c3c25203db110d279580c148b
* Changed 'OpenStack, LLC' message to 'OpenStack Foundation'Kurt Taylor2013-01-071-1/+1
| | | | Change-Id: Iffad63690d2d7565c651ff3faec4443d0ed471c3
* Ban db import from nova/virtDan Smith2012-11-121-0/+1
| | | | | | | | | | | | | | This makes hacking.py complain if someone adds something like: from nova import db into any of the files in nova/virt/* (aside from the fake.py file). It also removes the rest of the dangling db imports that are no longer needed. Yay! Change-Id: Iba3d53b87e65e33a55f8e5033b5d1d33b28d12f7
* hacking: Add driver prefix recommendation.Rick Harris2012-08-291-0/+4
| | | | | | | | | Prefixing the commit message's first line with the driver makes it easier for reviewers, at a glance, to find reviews that relate to their area of expertise as well as improving the overall readability of the git history. Change-Id: I376d2cdb5dc344717fb7749a80e33ee88603e68c
* OpenStack capitalization added to HACKING.rstJoe Gordon2012-08-171-0/+9
| | | | | | Along with capitalization fixes to comments in code Change-Id: I72ddc582001f80d954ca5a121903c689f40d08d1
* Add a 50 char git title limit test to hacking.Joe Gordon2012-08-081-1/+2
| | | | | | * add N802 to hacking.py Change-Id: I8262531b4b3f8f3a1a2a37679904cf4864cae7b6
* Add a link from HACKING to wiki GitCommitMessages pageDaniel P. Berrange2012-07-301-0/+6
| | | | | | | | | | | | | | The HACKING file contains a short example of an effective commit message. For reasons for space, it cannot describe the rationale behind this example. It also does not have space to describe how to split up a patch into a series of commits. Add a link from the HACKING file to the wiki http://wiki.openstack.org/GitCommitMessages where further information can be found Change-Id: I34d44485486b623b11743106f09d5ef631d35888
View Lorry Controller Status