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.

Sign up for GitHub

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

Jump to bottom

add GerritChangeFilter #1070

Merged
merged 4 commits into from May 6, 2014
Merged

add GerritChangeFilter #1070

merged 4 commits into from May 6, 2014

Conversation

tardyp
Copy link
Member

@tardyp tardyp commented Feb 18, 2014

I realize I never upstreamed this simple change filter I made two years ago.

Hoping this will help to understand how things are supposed to be bound together.

Its not very clear for people how to use gerritchangesource to filter
gerrit change events, and create builds.

add GerritChangeFilter, which is a simple general purpose filter, which
manage most of the changefilters needs

doc + tests

and some whitespace fixing in the doc.

@tardyp
Copy link
Member Author

tardyp commented Feb 18, 2014

@hgenru @PiotrSikora comments?

@djmitche
Copy link
Member

I'd like to hear comments from someone else using Gerrit, but I have no problem with this - please merge when you have feedback, or if none comes.

@PiotrSikora
Copy link
Contributor

Sorry, I've stopped using buildbot (and Gerrit) a while ago.

@hgenru
Copy link
Contributor

hgenru commented Feb 25, 2014

What about ChangeFilter validation functions?

add GerritChangeFilter
8c205b0 
Its not very clear for people how to use gerritchangesource to filter
gerrit change events, and create builds.

add GerritChangeFilter, which is a simple general purpose filter, which
manage most of the changefilters needs

doc + tests

and some whitespace fixing in the doc.

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
@tardyp
Copy link
Member Author

tardyp commented Feb 25, 2014

new version with reuse of the original changefilter

@hgenru
Copy link
Contributor

hgenru commented Feb 25, 2014

Will this work with Nightly builds? Now there are some difficulties.

@tardyp
Copy link
Member Author

tardyp commented Feb 25, 2014

Well, if you specify what kind of difficulties, I could tell. I have made
Nightly builds with this for 3 years.

Pierre

On Tue, Feb 25, 2014 at 9:41 AM, Alexander Plesovskikh <
notifications@github.com> wrote:

Will this work with Nightly builds? Now there are some difficulties.

Reply to this email directly or view it on GitHubhttps://github.com//pull/1070#issuecomment-35985958
.

@hgenru
Copy link
Contributor

hgenru commented Feb 25, 2014

https://github.com/buildbot/buildbot/blob/master/master/buildbot/schedulers/timed.py#L348
I think the problem may be in this line.

master =! master/1234
see: https://github.com/buildbot/buildbot/blob/master/master/buildbot/test/unit/test_changes_gerritchangesource.py#L55

@tardyp
Copy link
Member Author

tardyp commented Feb 25, 2014

indeed. I think Nightly should use ChangeFilters instead.

Pierre

On Tue, Feb 25, 2014 at 10:19 AM, Alexander Plesovskikh <
notifications@github.com> wrote:

https://github.com/buildbot/buildbot/blob/master/master/buildbot/schedulers/timed.py#L348
I think the problem may be in this line.

master =! master/1234
see:
https://github.com/buildbot/buildbot/blob/master/master/buildbot/test/unit/test_changes_gerritchangesource.py#L55

Reply to this email directly or view it on GitHubhttps://github.com//pull/1070#issuecomment-35988836
.

@hgenru
Copy link
Contributor

hgenru commented Feb 25, 2014

They use, but somehow wrong.

Pierre Tardy added 3 commits March 2, 2014 15:43
gerritfilterbranch: more factorization
fdda0cb 
This comes at the prize of a little bit of complexity

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
gerritchangesource: fix grouping for Nightly schedulers
cb2b2ef 
for nightly and post-commit style schedulers,
you really need to have the branch to be the real gerrit branch
or the list of changes in the UI will be incorrect

This is a simple way of implementing it, probably by separating the
change grouping to the notion of change branch

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
gerritchangesource: test branch is branch for event type != changeset…
4164c6a 
…-created

and some cleanup of failUnless* API usage

Signed-off-by: Pierre Tardy <pierre.tardy@intel.com>
@tardyp
Copy link
Member Author

tardyp commented Mar 2, 2014

updated the branch with some improvments, according to the comments.
@hgenru please tell if this would works in your env for nightlies
@krf, I did not go up to the callable customization. The problem of this is that it would become an api, and I dont think it is really needed.

@djmitche
Copy link
Member

djmitche commented Apr 7, 2014

@tardyp if you're happy with this, then in the absence of comment from @krf and @hgenru, go ahead and merge.

@hgenru
Copy link
Contributor

hgenru commented Apr 7, 2014

Excuse me. I now have no way to verify.

@djmitche
Copy link
Member

djmitche commented May 5, 2014

@tardyp please merge

@tardyp tardyp merged commit 4164c6a into buildbot:master May 6, 2014
@tardyp tardyp deleted the gerrit_change_filter branch May 6, 2014 07:51
@tardyp
Copy link
Member Author

tardyp commented May 6, 2014

merged in master and nine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tardyp @djmitche @PiotrSikora @hgenru @krf @yurboc