Skip to content
This repository has been archived by the owner on Jul 24, 2018. It is now read-only.

create new fmn rules #21

Merged
merged 27 commits into from Nov 21, 2014
Merged

create new fmn rules #21

merged 27 commits into from Nov 21, 2014

Conversation

sayanchowdhury
Copy link
Contributor

No description provided.

@sayanchowdhury
Copy link
Contributor Author

Fixes fedora-infra/fmn#31

return message['topic'].endswith('anitya.project.add.tried')


def anitya_project_update(config, message):
Copy link
Contributor

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
Copy link
Contributor

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

See fedora-infra/fedmsg_meta_fedora_infrastructure#172.



def fedoratagger_usage_toggle(config, message):
""" Tagger: Someone voted on a tag
Copy link
Contributor

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')
Copy link
Contributor

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):
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@ralphbean
Copy link
Contributor

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

@pypingou
Copy link
Member

@ralphbean I see changes against anitya and pkgdb, please give me a little time to go through them before merging :)

@ralphbean
Copy link
Contributor

please give me a little time to go through them before merging

Sure thing!

@pypingou
Copy link
Member

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

@ralphbean
Copy link
Contributor

Travis seems un-happy though

Yeah, @sayanchowdhury there's a test failure here: https://travis-ci.org/fedora-infra/fmn.rules/jobs/41623452

@sayanchowdhury
Copy link
Contributor Author

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

@pypingou
Copy link
Member

@sayanchowdhury that could indeed be a problem, maybe rename it to fmn_notifications.py or so?

@sayanchowdhury
Copy link
Contributor Author

@pypingou renamed the file to fmn_notifications.py

@pypingou
Copy link
Member

And travis is happy again :)

@ralphbean
Copy link
Contributor

👍 to merge here. Thanks both @sayanchowdhury and @pypingou!

ralphbean added a commit that referenced this pull request Nov 21, 2014
@ralphbean ralphbean merged commit 8cb2ca6 into fedora-infra:develop Nov 21, 2014
ralphbean added a commit to fedora-infra/fmn.lib that referenced this pull request Jan 8, 2015
In fedora-infra/fmn.rules#21 a few rules got removed.  This new alembic
upgrade script goes through and removes them from the db.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants