Feature/add support for TickerQ#3903
Conversation
|
Thanks @AboubakrNasef. I'll ping @ lillo42 as his review is important to me. |
|
Looks like a build error from a typo: |
|
@lillo42 If you give me the thumbs up, I can approve |
|
Looking good @AboubakrNasef. Just a couple of niggles to fix up. |
|
@AboubakrNasef If you can fix up the files (licences etc.) we can merge this |
im not sure how to do so |
No worries, you have given us a contribution, so we are the ones who should be thankful. So, you need to add a licence header to your source files (don't worry about the tests). You can see what you need to add here: https://docs.google.com/document/d/1H_AlNNiQK5645NLYCtGfEJs21FAe8Zj5bGxOks0F2Ew/edit?tab=t.0#heading=h.lewf11cspa2r. It goes at the top of the file, and you need to add your name in the copyright. (Under the CLA you kept your copyright but granted us a perpetual licence) If you look in the comments, there is a #region in TickerQScheduler.cs. Take that out If you look in the comments, there is a suggestion to wrap ScheduleAsync rather than wrapping the one call within Schedule. You have done this elsewhere; use an expression-bodied function and call BrighterAsyncContext.Run(async () => await ScheduleAsync ... |
|
Still not clear? Just shout. |
…ubakrNasef/Brighter into feat/Add-Support-for-TickerQ
There was a problem hiding this comment.
Gates Passed
4 Quality Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
|
i've added the licensce now |
for this suggestion i cant agree with because of the creation of the FireSchedulerMessage and FireSchedulerRequest |
|
Thanks @AboubakrNasef I am going to merge now. We are very grateful for your contribution. I hope to release a new version this weekend which should include this. We always appreciate contributions, please keep the coming. |
|
Hey @AboubakrNasef can you create a PR with version 9 of Ticker? |
|
@lillo42 |
|
No, I mean, you have implemented the TicketQ using the version |
|
ok ive checked they just updated the versions |
|
@AboubakrNasef No worries, I need to get a build out, and due to a vulnerability, I am fixing it. I'll let you know when it is done, and you can review my changes |
|
@iancooper i did start on this already they have some breaking changes they did |
|
|
@AboubakrNasef BTW, I am moving to their version 8, as they seem to have dropped support for all but 10 in their latest release. |
* add tickerq and test project * add TickerQ library * add test for schedule message * Implement the Schedule Message Functions * add request shceduler messages WIP * add documentation * refactor test fixtures * pass the cacnelation token * refactor the creation logic of the timeTicker * fix typo * remove launch settings * use the Id instead of guid * add the strong namer for the TickerQ.Utilities isn't signed with strong name * remove properites folder * update the package to not Sign the assembly * remove space from file serviceActivator api * Remove the AckOnRead flag in AzureServiceBusConfiguration (BrighterCommand#3899) See BrighterCommand#3895 * Fixes found bug on Messaging Gateway (BrighterCommand#3905) * Fixes found bug on Messaging Gateway * fixes build * Make RedisSubscription constructor as public (BrighterCommand#3906) * add MIT License to files --------- Co-authored-by: Doğan Çeçen <sepeth@hey.com> Co-authored-by: Rafael Lillo <rafael.andrade@justeattakeaway.com> Co-authored-by: Ian Cooper <ian_hammond_cooper@yahoo.co.uk>
[Feature] Add Support for TickerQ #3691