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

scripts: new backport-create-issue script #21480

Merged
merged 3 commits into from Apr 18, 2018

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented Apr 17, 2018

This is an initial (and rather careless) standalone implementation of a script written by Loic Dachary. It has been tested manually, and seems to do the job.

The script iterates over all issues in status "Pending Backport" and, where necessary, creates issues in the Backport tracker and "Copied to/from" relations between the backport tracker issue and the parent issue.

I only tagged @ktdreyer and @theanalyst for review, but all reviews are welcome!!!

@smithfarm smithfarm force-pushed the wip-backport-create-issue branch 2 times, most recently from d45d56e to 8ac959e Compare April 17, 2018 19:31
This is a standalone implementation of a script written by Loic Dachary.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
return None


args = parse_arguments()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might want to guard following statements with

if __name__ == '__main__':

it's a useful idiom in python development. it helps when one wants to debug part of this script without running all the main() statements in a wholesale.

update_relations(r, issue, dry_run)
logging.info("Processed {} issues with status Pending Backport"
.format(counter))
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this return value is never used. and a method return nothing actually evaluates to None. so this is not necessary.

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 prefer to make the return value explicit. When I'm re-reading the code, it signals to me that this function is not designed to return anything useful.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I support not returning, I'm ok just merging this as is as this should beat copying issues in tracker that many users get wrong anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@theanalyst Thanks! The reasoning for making the return value explicit is (a) it does something useful (remind me that the function is only used for its side effects, not for its return value), and (b) it does no harm. But this is a matter of coding style preference.

logging.basicConfig(level=logging.DEBUG)
else:
logging.basicConfig(level=logging.INFO)
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to return "None".

Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm smithfarm changed the title scripts: backport-create-issue (initial commit) scripts: new backport-create-issue script Apr 18, 2018
@smithfarm smithfarm merged commit 90f5424 into ceph:master Apr 18, 2018
@smithfarm smithfarm deleted the wip-backport-create-issue branch April 18, 2018 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants