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

Fixes for migrating comments #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GeoffreyBooth
Copy link

I was having two issues with gh-issue-mover:

  1. Issues would migrate with only the first 30 comments.
  2. Those comments would be out of order.

This PR fixes both issues.

The reason only 30 comments were getting migrated was because comments weren’t being paginated; now they are.

After I got that fixed, I noticed that an exception would always be thrown after exactly 35 comments, which currently happens on Google’s Issue Mover. This was because 35 comments within a short period was triggering GitHub’s abuse detection API. To compensate for this, I space out the posts by a second each, and wait a minute after each 403 returned from GitHub (the status code for the abuse rate limit).

I’m also using a regular for loop, rather than .map, to loop through the comments to add them to the new issue. This preserves the order by ensuring they’re posted sequentially, because we wait for each comment to finish posting before starting to post the next one.

Finally, I changed the formatting of the comment header to more closely resemble Google’s, but with keeping the link back to the source comment and using locale-agnostic date formatting (that’s not too verbose).

…e limiting

- Paginate comments, to avoid posting only the first 30
- Preserve the order of comments when reposting them in the new issue
- Format comment headers more like Google’s GitHub Issue Mover
- Improve status output during migration
- Preserve status of migrated issue
- Bump version, add package-lock.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants