Skip to content
This repository was archived by the owner on Sep 23, 2024. It is now read-only.

Conversation

@spbnick
Copy link
Contributor

@spbnick spbnick commented May 28, 2018

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.

@spbnick
Copy link
Contributor Author

spbnick commented May 28, 2018

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 */
Copy link
Contributor

@veruu veruu May 28, 2018

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.

Copy link
Contributor Author

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.

@veruu
Copy link
Contributor

veruu commented May 28, 2018

I finally went through everything and what you have so far looks good! :)

return r.json()

def get_patchsets_by_patch(self, url, db=None, seen=set()):
def get_patchsets_by_patch(self, url, seen=set()):
Copy link
Contributor

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?

Copy link
Contributor Author

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 :)

Copy link
Contributor

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.

@spbnick spbnick force-pushed the support_patchset_filtering branch 2 times, most recently from 62242f0 to afb092b Compare May 31, 2018 11:20
@spbnick spbnick mentioned this pull request May 31, 2018
1 task
@spbnick spbnick force-pushed the support_patchset_filtering branch from afb092b to 4ebf552 Compare May 31, 2018 11:25
@spbnick
Copy link
Contributor Author

spbnick commented May 31, 2018

Split off patchset delaying. This is just basic filtering now.

@spbnick spbnick force-pushed the support_patchset_filtering branch 2 times, most recently from 6eecc29 to 0fa5c1f Compare May 31, 2018 12:10
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.
@spbnick spbnick force-pushed the support_patchset_filtering branch from 0fa5c1f to 31eed92 Compare May 31, 2018 12:17
spbnick added 2 commits May 31, 2018 15:21
Add support for specifying the cover letter ObjectSummary in
patchwork.PatchsetSummary to support passing cover letter URLs to the
future patch filtering routine.
@spbnick spbnick force-pushed the support_patchset_filtering branch from 31eed92 to 5de1b74 Compare May 31, 2018 12:22
@spbnick spbnick changed the title WIP: Support patchset filtering Support patchset filtering May 31, 2018
@spbnick
Copy link
Contributor Author

spbnick commented May 31, 2018

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 :)

@spbnick spbnick requested a review from major May 31, 2018 12:25
@major major added the enhancement New feature or request label May 31, 2018
@major
Copy link
Contributor

major commented May 31, 2018

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. ;)

@major major merged commit 4756bab into cki-project:master May 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants