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
Phabricator service: Adding option to filter on users and projects #216
Conversation
This patch adds phabricator.user_phids and phabricator.project_phids options which, when specified, filter the results to match only the tasks and diffs that match the specified users and projects/repositories.
Damn github modifies original pull request after pushing more commits to my fork... I've changed the title and the description to reflect the cumulative changes. p.s. The build issues reported by Travis do not seem to be related to this patch. |
@ivan-cukic generally it's recommended that you create a branch for submitting changes, that way you can continue working on other issues and submit multiple pull requests -- one per issue -- without worrying about them interfering with one another. |
That said -- since these are all in just this module, don't feel a need to do that, just an FYI. |
@coddingtonbear Yeah, I got that from the auto-updates. I'm more used to gerrit and revboard workflow, github is sometimes like a strange beast. :) |
Looks good! I'm not sure if you noticed that there were documentation files in the repo (see https://github.com/ralphbean/bugwarrior/blob/develop/bugwarrior/docs/services/phabricator.rst) for each service -- these are how our online documentation at http://bugwarrior.readthedocs.org/ is generated. Could I convince you into updating those docs to include some instructions to users using those configuration settings? If you need an example to follow: the github service documentation documents a couple extra configuration values in a section named "Service Features". Cheers! |
I waited for the green light for the feature before writing the docs (I mentioned it in the fist comment). No need to convince me, it will be done. :) |
Oh! Consider yourself green-lit. Although it's very considerate of you to check-in about per-service features before spending time writing them, generally as long as you aren't altering the functionality of modules in a general sense or breaking backward compatibility you're good-to-go. |
This should be it. I kinda got used to ask before working on something. It is not pleasant when you are the maintainer of something, and you need to reject a patch because you are against the proposed feature, or when it is implemented in a wrong way. :) |
Totally! Nobody likes to waste their time; it's appreciated!
|
@coddingtonbear Do you have merge rights? |
I do, but I think @ralphbean would rather that I let him have a look first.
|
Ok, thanks! |
LGTM. Thanks both! FWIW, in the future Adam you have my permission to merge if you feel things are all set. |
Phabricator service: Adding option to filter on users and projects
WIP.
The first patch fixes the issue of the python module for phabricator service being called phab, and not phabricator.
The second introduces options to filter out the phabricator tasks and diffs according to the project and the user. (the phabricator service is not really useful now when you have a phabricator setup with many users and projects)
If there is the interest in merging this to mainline, I'll write the documentation for the new options.