-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
|
Oh, and database documentation needs to be finished eventually. |
sktm/db.py
Outdated
| baseurl TEXT, | ||
| /* Numeric Patchwork project ID */ | ||
| project_id INTEGER, | ||
| /* TODO Document or remove */ |
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.
Checking git history, this seems to be a residue of adding events API support and neither date nor the set_event_date and get_last_event_date methods which use it aren't used anymore. Now the nsince attribute is used to save these information. I'm fine with eventual removal of the column and methods that use it.
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.
OK, cool :) I'll change this to just TODO: Remove.
|
I finally went through everything and what you have so far looks good! :) |
sktm/patchwork.py
Outdated
| return r.json() | ||
|
|
||
| def get_patchsets_by_patch(self, url, db=None, seen=set()): | ||
| def get_patchsets_by_patch(self, url, seen=set()): |
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.
Do you want to have this in this pull or should I put it into my patchwork pull?
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 a side-effect of me considering if we should access database in Patchwork, and seeing some stuff there I thought we'd better distance ourselves from that idea. It's not at all needed for adding filtering, so if it's better for you, take it over, and I'll remove it from here :)
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.
That's what I thought :) I'll add it to the pull.
62242f0 to
afb092b
Compare
afb092b to
4ebf552
Compare
|
Split off patchset delaying. This is just basic filtering now. |
6eecc29 to
0fa5c1f
Compare
Implement patchwork.ObjectSummary class representing a summary of any mbox-based Patchwork object, storing the object URL, "Date" header, and patch ID (for patch objects). Switch to storing a list of instances of that class in patchwork.PatchsetSummary, instead of just URLs, to represent the patches comprising the patchset. This avoids having to re-request all the patches in the watcher only to get their dates and IDs, and prepares the code for the addition of cover letter support and delayed patches.
0fa5c1f to
31eed92
Compare
Add support for specifying the cover letter ObjectSummary in patchwork.PatchsetSummary to support passing cover letter URLs to the future patch filtering routine.
31eed92 to
5de1b74
Compare
|
OK, I think this is in a decent shape now. @major, I would really appreciate your review of this, my eyes are glazing over after spending so much time with it :) |
|
No objections here. There are a few cleanups/simplifications we could do later, but the logic looks sound. I haven't tested it myself, though. ;) |
This is a work-in-progress support for filtering patchsets. Nothing here was tested, filtering implementation itself is missing, as well as adding cover letter URL to patchset summaries.