You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, the main classes (Job, Queue, Worker) have different types of methods interlaced. The Job for example, starts with a few classmethods, then a few methods, then some properties, then more classmethods, then the initializer and some dunder methods, and then more classmethods, and more methods, and then more properties (roughly the current order). One could argue that the properties should be closer to the methods using it, but properties are akin to an attribute, and the same way one would scroll up to look into attributes, IMO it's worth to keep them on top for the same reason. The same thing happens on other classes.
I understand this may seem just a cosmetic aspect, but IMHO it greatly changes how easy it is to reason about the codebase, and business logic and the flow of the program, hence dramatically improving maintainability and how easy it is for new contributors to understand what's going on.
My suggestion for a (sort of) standard is:
Static class attributes
The initializer __init__
All @properties
All methods, organized and split by the business logic
All classmethods
All __dunder__ methods (except the initializer)
The downside of this, is that is the sort of PR that is very hard to spot differences (although I made a suggestion on how to reduce clutter on my PR #1784), and introduces changes for those who are accustomed with the current codebase. A couple of other options include (1) very small granular PRs, with the downside of many PR's or (2) very small granular commits - multiple per file, in a way that the whole PR diff would be big, but the commit diff would be small.
I would love to work on this, IMHO is a very quick (and big) win on codebase organization.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
Currently, the main classes (
Job
,Queue
,Worker
) have different types of methods interlaced. TheJob
for example, starts with a fewclassmethods
, then a fewmethods
, then someproperties
, then moreclassmethods
, then theinitializer
and somedunder
methods, and then moreclassmethods
, and moremethods
, and then moreproperties
(roughly the current order). One could argue that the properties should be closer to the methods using it, but properties are akin to an attribute, and the same way one would scroll up to look into attributes, IMO it's worth to keep them on top for the same reason. The same thing happens on other classes.I understand this may seem just a cosmetic aspect, but IMHO it greatly changes how easy it is to reason about the codebase, and business logic and the flow of the program, hence dramatically improving maintainability and how easy it is for new contributors to understand what's going on.
My suggestion for a (sort of) standard is:
__init__
@properties
classmethods
__dunder__
methods (except the initializer)The downside of this, is that is the sort of PR that is very hard to spot differences (although I made a suggestion on how to reduce clutter on my PR #1784), and introduces changes for those who are accustomed with the current codebase. A couple of other options include (1) very small granular PRs, with the downside of many PR's or (2) very small granular commits - multiple per file, in a way that the whole PR diff would be big, but the commit diff would be small.
I would love to work on this, IMHO is a very quick (and big) win on codebase organization.
Beta Was this translation helpful? Give feedback.
All reactions