Skip to content
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

Merged
merged 4 commits into from Apr 20, 2015
Merged

Conversation

ivan-cukic
Copy link
Contributor

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.

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.
@ivan-cukic ivan-cukic changed the title Phabricator service module is not called phabricator, but phab Phabricator service: Adding option to filter on users and projects Apr 16, 2015
@ivan-cukic
Copy link
Contributor Author

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.

@coddingtonbear
Copy link
Collaborator

@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.

@coddingtonbear
Copy link
Collaborator

That said -- since these are all in just this module, don't feel a need to do that, just an FYI.

@ivan-cukic
Copy link
Contributor Author

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

@coddingtonbear
Copy link
Collaborator

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!

@ivan-cukic
Copy link
Contributor Author

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

@coddingtonbear
Copy link
Collaborator

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.

@ivan-cukic
Copy link
Contributor Author

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

@coddingtonbear
Copy link
Collaborator

Totally! Nobody likes to waste their time; it's appreciated!

On Apr 18, 2015, at 12:44, Ivan Čukić notifications@github.com wrote:

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


Reply to this email directly or view it on GitHub.

@ivan-cukic
Copy link
Contributor Author

@coddingtonbear Do you have merge rights?

@coddingtonbear
Copy link
Collaborator

I do, but I think @ralphbean would rather that I let him have a look first.

On Apr 19, 2015, at 00:26, Ivan Čukić notifications@github.com wrote:

@coddingtonbear Do you have merge rights?


Reply to this email directly or view it on GitHub.

@ivan-cukic
Copy link
Contributor Author

Ok, thanks!

@ralphbean
Copy link
Sponsor Collaborator

LGTM. Thanks both!

FWIW, in the future Adam you have my permission to merge if you feel things are all set.

ralphbean added a commit that referenced this pull request Apr 20, 2015
Phabricator service: Adding option to filter on users and projects
@ralphbean ralphbean merged commit 1f1f4f0 into GothenburgBitFactory:develop Apr 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants