-
Notifications
You must be signed in to change notification settings - Fork 18
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
[Server v2] Various improvements #317
Conversation
7ccc3cf
to
dd1d67a
Compare
API/Schema/Chapter.cs
Outdated
@@ -89,11 +100,11 @@ public bool IsDownloaded() | |||
|
|||
private static int CompareChapterNumbers(string ch1, string ch2) | |||
{ | |||
var ch1Arr = ch1.Split('.').Select(c => int.Parse(c)).ToArray(); | |||
var ch2Arr = ch2.Split('.').Select(c => int.Parse(c)).ToArray(); | |||
int[] ch1Arr = ch1.Split('.').Select(c => int.Parse(c)).ToArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should use TryParse
instead, unless you can ensure that the strings are definitely in the expected format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH String chapters are still a work in progress...
I started them, then got derailed by the jobs looping problem, and now I'm fixing the ID part.
As soon as I finish IDs I promise I'll get back at chapter numbers, fix and test all this
API/Tranga.cs
Outdated
if (notifications.Any()) | ||
{ | ||
var max = notifications.MaxBy(n => n.Date)!.Date; | ||
DateTime max = notifications.MaxBy(n => n.Date)!.Date; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, PGSQL does UTC Times only. So whenever we do comparisons, better use UTCNow
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this was due to the formatter config, sorry! I'll fix it ASAP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was just general information, nothing to fix! 👍🏼
Let's move #167 conversation to here instead 👍🏼 |
6a8df2f |
Considering the column size of 64, my solution takes 32 char for the token and leaves 32 for the eventual prefix |
Just noticed I'm missing the prefix concatenation after the token geneation but otherwise seems to work |
7dc34c1
to
be6b3da
Compare
Had some troubles with syncing your commits, I think it's best if I make (and merge) stuff when ready, otherwise conflicts are bound to happen |
No description provided.