Conversation
Fixes fedora-infra/fmn#31 |
return message['topic'].endswith('anitya.project.add.tried') | ||
|
||
|
||
def anitya_project_update(config, message): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the name of this function back to anitya_info_update
just so we have as few function name changes as possible.
def anitya_info_update(config, message): | ||
""" Anitya: a project has been updated | ||
def anitya_project_add_tried(config, message): | ||
""" Upstream: a project is tried to be added |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text in the docstring is a little unclear (probably because it was unclear in the fedmsg docs also)..
Can you change it to read something like 'someone tries to add a project to anitya that is already there'.
|
||
|
||
def fedoratagger_usage_toggle(config, message): | ||
""" Tagger: Someone voted on a tag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should read something like 'Tagger: Someone marked that they use a package'
from `Github <https://apps.fedoraproject.org/github2fedmsg>`_ | ||
when someone **deletes a tag or branch**. | ||
""" | ||
return message['topic'].endswith('github.create') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The topic check is wrong here. It should be something about deleting.
@@ -8,24 +8,24 @@ def pkgdb_acl_update(config, message): | |||
return message['topic'].endswith('pkgdb.acl.update') | |||
|
|||
|
|||
def pkgdb_acl_user_remove(config, message): | |||
""" Pkgdb: a user removed an ACL | |||
def pkgdb_acl_delete(config, message): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this one might be mixed up. deleting an acl and deleting a branch are not the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the function as it was not in the topics list. I removed few other functions too. Do i add them again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, no -- removing them is fine if its not in the topics list.
But, for this one, compare the function name and what it does against the docstring. I don't think they match up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies! I missed this message. I will look into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ralphbean i have changed the description in the last commit
|
||
|
||
def pkgdb_package_monitor_update(config, message): | ||
""" Pkgdb: a new branch is created for a package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring for this monitoring rule is incorrect -- it reads about branches being creating when it should be about the upstream monitoring status being changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description is wrong is the topics documentation. I will send a fix.
@sayanchowdhury thanks for the followup on this. I think there's only one or two more leftover little changes that need to be made before we can merge it into |
@ralphbean I see changes against anitya and pkgdb, please give me a little time to go through them before merging :) |
Sure thing! |
Ok looks fine to me, lots of pkgdb1 related topics dropped and the changes in anitya looks ok although hard to find out as some blocks of code were moved around and some changed, others not :) Travis seems un-happy though |
Yeah, @sayanchowdhury there's a test failure here: https://travis-ci.org/fedora-infra/fmn.rules/jobs/41623452 |
@pypingou @ralphbean The tests were failing because i created a file called fmn, due to which it was unable to import. Do i change the file name to notifications? |
@sayanchowdhury that could indeed be a problem, maybe rename it to |
@pypingou renamed the file to fmn_notifications.py |
And travis is happy again :) |
👍 to merge here. Thanks both @sayanchowdhury and @pypingou! |
In fedora-infra/fmn.rules#21 a few rules got removed. This new alembic upgrade script goes through and removes them from the db.
No description provided.