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

Revise queues.md #805

Merged
merged 1 commit into from
May 10, 2023
Merged

Revise queues.md #805

merged 1 commit into from
May 10, 2023

Conversation

mattesmohr
Copy link
Sponsor Member

This PR revises some bits of the queues.md, wich come to my attention while working with the library.

@mattesmohr
Copy link
Sponsor Member Author

mattesmohr commented Apr 30, 2023

I have added the import to the config snippet, without mention it in a tooltip to avoid the need of translations ^^.

@0xTim Do we update the package declaration example from time to time, or only when it's a major? The package is currently by version 1.0.4.

@mattesmohr
Copy link
Sponsor Member Author

In my opinion, that can be simplified

//Register jobs
let emailJob = EmailJob()
app.queues.add(emailJob)

to

//Register jobs
app.queues.add(EmailJob())

Or is there a reason to do it the way it is?

@mattesmohr
Copy link
Sponsor Member Author

mattesmohr commented May 1, 2023

The concurrency version of the jobeventdelegate is not mentioned

struct JobEventDelegate: AsyncJobEventDelegate {
    
    func dispatched(job: JobEventData) async throws {
    }

    func didDequeue(jobId: String) async throws {
    }

    func success(jobId: String) async throws {
    }

    func error(jobId: String, error: Error) async throws {
    }
}

@0xTim
Copy link
Member

0xTim commented May 10, 2023

@mattesmohr nah there's no need to bump the versions unless the docs refer to something specifically in a later version that could cause compiler issues.

Re splitting out the job, mainly helpful when you want to pass in stuff, but it's personal preference for an empty initialiser so it can be left.

Re the concurrency stuff, we should probably add that - feel free to do it as a separate PR!

@0xTim 0xTim added the no-translation-needed This PR does not require the translations to be updated (e.g. fixing a typo or infrastructure work) label May 10, 2023
@0xTim 0xTim merged commit 772f816 into main May 10, 2023
@0xTim 0xTim deleted the revise-queue-md branch May 10, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-translation-needed This PR does not require the translations to be updated (e.g. fixing a typo or infrastructure work)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants