Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: split magic numbers #139

Merged
merged 1 commit into from
Nov 28, 2022
Merged

doc: split magic numbers #139

merged 1 commit into from
Nov 28, 2022

Conversation

jnns
Copy link
Contributor

@jnns jnns commented Nov 23, 2022

In dramatiq, DEFAULT_RESULT_TTL = 600_000 which is documented as the 10 minute default. In django_dramatiq, the example uses "result_ttl": 60_000 which equates to 1 minute. Although the example shows a customization and not necessarily the defaults, the duration is better communicated by splitting it up into different units.

Maybe it's just me, but I wasn't sure which units the durations had to be specified in and had to look at the source code / docs.

In dramatiq, `DEFAULT_RESULT_TTL = 600_000` which is documented as the 10 minute default. In django_dramatiq, the example uses `"result_ttl": 60_000` which equates to 1 minute.  
Although the example shows a customization and not necessarily the defaults, the duration is better communicated by splitting it up into different units. 

Maybe it's just me, but I wasn't sure which units the durations had to be specified in and had to look at the source code / docs.
Copy link
Collaborator

@amureki amureki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @jnns !

Thanks for this improvement here! I agree, split number is better perceived.

@amureki amureki merged commit a49f6ce into Bogdanp:master Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants