Opened 3 years ago

Closed 3 years ago

#3950 closed change (fixed)

Update style guide to use 4 space indents for Python and some other practices

Reported by: kvas Assignee: sebastian
Priority: P3 Milestone:
Module: Websites Keywords:
Cc: saroyanm, sebastian Blocked By:
Blocking: #3949 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29340492

Description (last modified by sebastian)

Background

Our style guide recommends 2 space indents for all code, including Python code. This is in contradiction to PEP8 recommendation. So we recently agreed to move to 4 spaces for Python code, while sticking to 2 spaces for other languages, just like Mozilla does.

There are also a few other unspoken rules that we normally follow but that are not in the style guide. It would shorten code review discussions and make them more objective if we add such things to the style guide.

What to change

  • Change the style guide to just follow Mozilla style guide for formatting. We already do that but override indentation in our style guide. Since we choose to follow PEP8 for Python, we don't need the override.
  • Specifically mention PEP8 in the Python section of the style guide.
  • Add a point about string concatenation and formatting (+ to be used only for 2 strings, .join and .format preferred for more).
  • Add a point about choosing between using tuples and lists (based on https://docs.python.org/2/tutorial/datastructures.html#tuples-and-sequences).

Change History (5)

comment:1 Changed 3 years ago by kvas

  • Cc saroyanm sebastian added
  • Description modified (diff)

comment:2 Changed 3 years ago by sebastian

  • Description modified (diff)
  • Owner set to sebastian
  • Priority changed from Unknown to P3
  • Ready set
  • Summary changed from Change our style guide to allow 4 space indents for Python to Update style guide to use 4 space indents for Python and some other practices

comment:3 Changed 3 years ago by sebastian

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
https://hg.adblockplus.org/web.adblockplus.org/rev/8ab2e83850a6

comment:5 Changed 3 years ago by sebastian

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.