Opened on 04/18/2016 at 10:06:32 AM

Closed on 04/18/2016 at 12:19:24 PM

#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).

Attachments (0)

Change History (5)

comment:1 Changed on 04/18/2016 at 11:44:06 AM by kvas

  • Cc saroyanm sebastian added
  • Description modified (diff)

comment:2 Changed on 04/18/2016 at 11:50:52 AM 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 on 04/18/2016 at 11:52:04 AM by sebastian

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

comment:4 Changed on 04/18/2016 at 12:18:57 PM by abpbot

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

comment:5 Changed on 04/18/2016 at 12:19:24 PM by sebastian

  • Resolution set to fixed
  • Status changed from reviewing to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sebastian.
 
Note: See TracTickets for help on using tickets.