Skip to content

Archive-Epic: Added script to help moving project items. #143

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

minosgalanakis
Copy link

This this a simple utility script that allows migration of projects from the Epics Board to the Past Epics.

It wraps aroud the github cli so it requires a valid token. In some cases it has added complexity because the newly introduced gh project api is inconsistent on the argument requirements, and had to manually use graphQL querries to extract the information.

@gilles-peskine-arm
Copy link
Contributor

Does this still work after the GitHub board migration?

@minosgalanakis
Copy link
Author

I have not tried it to be honest, but we will definately need to retrofit it, since it was moving inbetween legacy to v2 type of projects.

Ideally we could replace it with something using pygithub, and it can be done in phases, since it was hacked around in a flat structure where each function does not depend to the others.

Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm left a comment

Choose a reason for hiding this comment

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

Thanks for sharing this! I've done a superficial review, and this mostly looks sensible to me, but there are two pain points in making this script accessible to people other than its author:

  • It's hard to get started with the script due to hard-coded assumptions.
  • Some parts (error handling, string escaping) seem fragile, so I don't know when I can trust the script and when I should be careful.

On the flip side, thanks for the documentation! Especially “User Config” and the explanation of how to find the magic strings project_xxx.

@@ -0,0 +1,340 @@
#!/usr/bin/env python3
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make the file executable.

from subprocess import Popen, check_output, PIPE
from pprint import pprint

token = os.environ['GITHUB_TOKEN']
Copy link
Contributor

Choose a reason for hiding this comment

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

Please support gh auth token if the environment variable is unset.

Also please arrange for mbedtls-archive-epic.py --help to work even when no token is available.

Comment on lines +46 to +51
epic = "3.6.1 patch release" # Name of epic that needs to be moved
# (It needs to match an empty epic on v2 board)
closed_after_date = "2024-01-01" # Only include entries closed after that date

max_entries = 500 # How many maximum entries the backend will populate.
debug = True # Set to True to print out the github cli commands.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these please be command line items? I don't want to have to modify the script to make it do something useful. Also, because there are prefilled default, I'm afraid that I'd forget to change something and I'd end up modifying something I didn't intend.

self.stderr = stderr


def do_shell_exec(exec_string, expected_result=0):
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this just be subprocess.check_output?

  • I don't see this being used for the error cases, except to print additional error messages that aren't particularly helpful.
  • Sometimes the caller forgets to check the return code, it would be better to keep using an exception on failure.
  • Callers don't actually rely on having a shell, they're just passing arguments to one program, sometimes with dodgy quoting. So it would be easier to not use a shell.

prs = get_issues_prs_for_project(issues=False, is_closed=is_closed, repo=repo)
output["prs"] = link_issues_pr_linked_to_epic(prs, column_name, projectboard)

# If both is selected just toggle the is_closed state and append the results
Copy link
Contributor

Choose a reason for hiding this comment

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

That seems a bit convoluted. Why not just omit both is:open and is:closed when status=="both"?

output = {"epic": column_name,
"issues": [], "prs": []}

if status not in ["open", "closed", "both"]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be an enum?

Or just not bother, since this function is always called with status="both"?

@@ -0,0 +1,340 @@
#!/usr/bin/env python3
""" Assist with the transition of a projectv1 to a projectv2.
Copy link
Contributor

Choose a reason for hiding this comment

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

Or between projectv2, surely? AFAICT this does work FROM v2 as well, and it's what we want going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: In Development
Development

Successfully merging this pull request may close these issues.

2 participants