Do these indents match the existing comments? If not the standard is to keep it the same indentation as the """

Small thing - is there a reason for the leading spaces?

You might get push back on removeAll vs remove_all

AFAIK this should be good enough provided users are required to enter their full name here since JIRA uses full names as display names. The only issue will be if an individual decides against user their own name. In that case - I'm not sure there's much you can do

It would be a bit easier to read if you used a string format here instead - roughly this:
description = '[{0}] Upstream Reporter: {1} \n{2}\n\n{3}'.format(today.strftime("%a %b %y - %H:%M"), issue.reporter['fullname'], issue.url, description)

Also make sure that whatever time is being produced by datetime.today() is in an appropriate timezone relative to everything else

If you want to avoid 2 loops here you could take the union of these two lists as sets:
list(set(jira_labels).union(set(issue_labels)))

You might want to catch the specific error that's thrown for this use case because there's both index and key access going on. This means if one of the values doesn't have an elements in it you'll get an IndexError or if the keys are missing you'd get a KeyError and they won't mean what you expect.

same as above

This should probably be a log.warning. Is there any action to take in this case?

This should probably be a warning and you might want to catch the specific error. Could the client have a connection issue? If it's possible - it's likely worth handling that

Generally if a function does not have any args and doesn't return the comment doesn't require them

log.warning or log.error maybe?

If this is a true failure you might want to exit(1) so that the user knows something wrong happened

You might want to use a more descriptive name

Small thing - is there a reason for the leading spaces?
Yes they help with the formatting of the logs (make it look way better and was formatted like this prior to me working on the project)

This should probably be a log.warning. Is there any action to take in this case?
Not sure if there is any action to be done as the user entered an invalid fix version, but will ask around and check.

I'd vote to remove the verify_JIRA work from this pull request, so that we can review and merge only the new sync stuff separate from the verification stuff. This is good practice in general - make smaller, isolated changes.

This looks new to me. Where does issue.downstream['updates'] get set?

Will this change in format result in old comments being disregarded and duplicates added to all currently managed jira issues?

This looks new to me. Where does issue.downstream['updates'] get set?

It will be set up in fedmsg.d/sync2jira.py. It will go under map, an example would be like this:
'Testing_initialization': {'project': 'FACTORY', 'component': 'gitbz', 'updates':
['comments', {'tags': {'overwrite': 'True'}}, {'fixVersion': {'overwrite': 'True'}},
'assignee', 'status']},
This gives the user the option to indicate if they want upstream repos to 'overwrite' JIRA issues.

Will this change in format result in old comments being disregarded and duplicates added to all currently managed jira issues?

Yes, this will. I have added code to ensure that duplicate comments are picked up

I'd vote to remove the verify_JIRA work from this pull request, so that we can review and merge only the new sync stuff separate from the verification stuff. This is good practice in general - make smaller, isolated changes.

I'll remove all verify_JIRA references

Hm, can you replace all this with raise TypeError("Got %s, expected Issue" % type(issue).__name__).

It is a type error, after all. Good catch.

Add more of a comment here about why priority is broken, otherwise in the future we'll be sad and confused.

Oh, this is a fedmsg config value - https://fedmsg.readthedocs.io/en/stable/configuration/#validate-signatures

Should remove this statement from your patch.

@sidpremkumar, can you remove the .idea/ folder from your patch and add .idea to the .gitignore file?

Also, please undo your changes in fedmsg.d/. I don't think those are necessary, right?

@sidpremkumar, can you remove the .idea/ folder from your patch and add .idea to the .gitignore file?
Also, please undo your changes in fedmsg.d/. I don't think those are necessary, right?

I've gone ahead and removed the .idea folder.

I've updated the fedmsg.d/sync2jira.py file to reflect the updates I've made (i.e. added a updates array in the example). I believe these are necessary but could also be seen in the updated README.md

I've updated the fedmsg.d/sync2jira.py file to reflect the updates I've made (i.e. added a updates array in the example). I believe these are necessary but could also be seen in the updated README.md

Ah, I see some other unrelated updates in there. Can you prune it down to only the necessary changes?

@sidpremkumar, I guess I'm really only concerned about this line. Can you revert this one?

@sidpremkumar, I guess I'm really only concerned about this line. Can you revert this one?

Done, ATM the sync2jira & tests are not working. I had some free time today so I've been working to reduce the number of API calls. Should be working by Friday.

Ack. Ok. GIve me a ping when you've got the tests working with tox and I'll give another review pass.

:-1: to this change. Please keep fedmsg.d/zz_sync2jira.py in the .gitignore.

Not sure you want to ignore fedmsg.d/sync2jira.py either.

Ha, still a typo here. :)

Can you revert this one too?

git commit --amend will alter your now nicely rebased commit.

Awesome. :+1:, let's merge this.

I'll monitor a deployment of it this afternoon.

@sidpremkumar, do you have rights to watch the logs of the prod instance?