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

ReviewBot & leaper: provide deduplicate method for leaper comment (and other fixes) #693

Merged
merged 5 commits into from Feb 17, 2017

Conversation

@jberry-suse
Copy link
Contributor

jberry-suse commented Feb 15, 2017

  • ReviewBot & leaper: provide deduplicate method for leaper comment.
  • leaper: utilize ReviewBot check_action__default() message.
  • ReviewBot: check_action__default() should return None not set ret.
  • leaper: drop extra definition of check_action__default().
  • ReviewBot: drop duplicate import namedtuple.

This resolves the duplicate line in comment on unhandled request types (like sr#457061).

Before:

unhandled request type delete

unhandled request type delete

request needs review by release management

After:

unhandled request type delete

request needs review by release management

I debated making the maintbot check conditional or similar, but seems like this can occur in a variety of ways and even more in the future. If maintbot ends up supporting different types of requests than leaper the message may be appropriate, etc. As such I implemented deduplicate via list(OrderedDict.fromkeys(self.comment_handler.lines)).

Additionally, there are two definitions of check_action__default() one after the other in leaper.py. The second one was added later and is the current behavior so I dropped the older one.

@jberry-suse jberry-suse force-pushed the jberry-suse:leaper-unhandled branch from 51c6d43 to 87cb5c2 Feb 15, 2017
@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 15, 2017

Coverage Status

Coverage remained the same at 45.475% when pulling 87cb5c2 on jberry-suse:leaper-unhandled into b3129e9 on openSUSE:master.

@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Feb 17, 2017

might be also worth rethinking how leaper uses other bots (or pieces of them). Maybe we can find a smarter way.

@lnussel lnussel merged commit c6f4f44 into openSUSE:master Feb 17, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jberry-suse jberry-suse deleted the jberry-suse:leaper-unhandled branch Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.