[doc] context must be offered as positional argument

[ticosax]

Closes #

Successful PR Checklist:

• [ ] Tests
  • [x] (not applicable?)
• [x] Documentation
  • [ ] (not applicable?)

PR label(s):

• [ ] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20breaking%20%F0%9F%92%A5
• [ ] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20feature%20%E2%AD%90%EF%B8%8F
• [ ] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20bugfix%20%F0%9F%95%B5%EF%B8%8F
• [ ] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20miscellaneous%20%F0%9F%91%BE
• [ ] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20dependencies%20%F0%9F%A4%96
• [x] https://github.com/procrastinate-org/procrastinate/labels/PR%20type%3A%20documentation%20%F0%9F%93%9A

[ewjoachim]

Hello,

Thank you for your contribution, can you spare a few words regarding the problem you're solving? The fact the job context is offered as a positional argument is a careful design decision as of today. We could imagine changing this decision (which would be a breaking change), but only if there are good reasons to do so.

The reasoning behind making it a positional argument is to distinguish it from custom task arguments which currently are always named. This way, there's no confusion possible between the job context and your arguments. Also, if you name one of your arguments job_context without it being THE job context, but just something that makes sense to you, you don't get an issue.

What would be the reasons for changing ? (It might not seem to be worth it for very small contributions like this one, but if you have anything bigger in mind, feel free to open an issue beforehand, so that we minimize the risk of you spending time on a PR that wouldn't match the direction of the repo)

[ticosax]

I'm trying to make the code snippet from the doc a bit more copy/paste friendly. The snippet from the doc doesn't work because context is passed as keyword argument. So the fix is to pass it as a positional one. I'm not trying to argue we must change the signature of the remove_old_jobs function.

[medihack]

@ticosax I am not directly against this change, but the snippet in the documentation should also work without your change. You can still pass the positional argument as a keyword argument, e.g. this is valid

def hello(foo: int, *, bar: int):
    print(foo, bar)

hello(bar=5, foo=3)

That said, maybe the change is a tad cleaner as it signalizes in the documentation that context is a positional argument, but it doesn't really matter 😉.

[ewjoachim]

Oooooh, I read too fast, sorry ! You're absolutely right ! The current doc snippet is probably not pyright compatible.

[ewjoachim]

Sorry again for the awkard review, thank you for your contribution !

[github-actions[bot]]

Coverage report

This PR does not seem to contain any modification to coverable code.

[ticosax]

Sorry again for the awkard review, thank you for your contribution !

you are welcome. I was not expecting this pull request to get that much attention. I'm sorry you had to spend time on it. I'll try to take it as a learning experience to provide a description from now on, even when the diff seems obvious ... to me

[ewjoachim]

I'm sorry you had to spend time on it.

On the contrary, I didn't spend long enough. Had I spent 1 minute reading it instead of wrongly assuming I had understood what was happening, I'd have avoided writing a few paragraphs and being an idiot :) But yeah, descriptions & context, even short, always help!