labhub.py: add migrate_issue plugin by aabhaas-vaish · Pull Request #521 · coala/corobo

aabhaas-vaish · 2018-04-07T15:56:04Z

Introduce migrate_issue plugin that adds ability
to migrate an issue from a source repo to target
repo, both owned by the org.

Issue title, issue description and all comments
are copied with a few additional details appended to
the the description and comments. Source issue is
referenced in the target issue and is closed after
the migration is complete.

Closes #518

Reviewers Checklist

Appropriate logging is done.
Appropriate error responses.
Handle every possible exception.
Make sure there is a docstring in the command functions. Hint: Lookout for
botcmd and re_botcmd decorators.
See that 100% coverage is there.
See to it that mocking is not done where it is not necessary.

meetmangukiya · 2018-04-07T16:49:26Z

                 )
        yield reply
+
+    @re_botcmd(pattern=r'^migrate\s+https://github.com/([^/]+)/([^/]+)/+issues/(\d+)\s+https://github.com/([^/]+)/([^/]+)/*$',  # Ignore LineLengthBear, PyCodeStyleBear


Please make this workable for gitlab repos as well. The whole point of labhub is to provide a unified front for both GitHub and GitLab, and hence the name LabHub 😉

meetmangukiya · 2018-04-07T16:51:12Z

+            yield 'Second repository not owned by our org.'
+            return
+
+        if (repo_name_orig in self.REPOS) and (repo_name_final in self.REPOS):


use, if not... so we can remove the reduntant if block and the contained pass statement

meetmangukiya · 2018-04-07T16:51:26Z

+            yield 'Repository does not exist!'
+            return
+
+        if self.TEAMS[self.GH_ORG_NAME + ' maintainers'].is_member(user):


Same as above

meetmangukiya · 2018-04-07T16:52:14Z

+            old_labels = old_issue.labels
+
+        except RuntimeError:
+            yield 'Issue does not exist!'


RuntimeError in IGitt is not always 404s 😅

meetmangukiya · 2018-04-07T16:53:02Z

+        if str(old_issue.state) == 'closed':
+            yield 'Issue cannot be migrated as it has been closed already.'
+            return
+        else:


else block is not needed, remove it

meetmangukiya · 2018-04-07T16:54:51Z

+        url1 = 'https://github.com/{}/{}/issues/{}'
+        new_issue_title = old_issue.title
+        new_issue_description = old_issue.description.rstrip()
+        extra_msg = '\n\nMigrated issue from '+url1.format(


maybe include the author as well? Migrate issue opened by @author from ... so the author the issue follows new issue as well

I was trying the same, but there seems to be some problem with the IGitt module I have installed on my system using pip, as it says author of the issue is not an attribute even though I find this method here: https://gitlab.com/gitmate/open-source/IGitt/blob/master/IGitt/GitHub/GitHubIssue.py and I am not particularly sure why but might be some problem with my system... 😅

meetmangukiya · 2018-04-07T16:55:48Z

+            ' by @' + str(user)
+
+        new_issue = self.REPOS[repo_name_final].create_issue(
+                    new_issue_title, new_issue_description+extra_msg)


Just concatenate this above, concatenation in function args doesn't look good and is confusing

meetmangukiya · 2018-04-07T16:57:11Z

+
+        old_comm = old_issue.comments
+
+        for i in range(len(old_comm)):


use for comment in old_issue.comments and get rid of few lines of code here 😉

gitmate-bot · 2018-04-11T11:34:54Z

+            yield 'Second repository not owned by our org.'
+            return
+
+        if (repo_name_orig not in self.REPOS) or (repo_name_final not in self.REPOS):


Line is longer than allowed. (85 > 80)

Origin: LineLengthBear, Section: all.linelength.

gitmate-bot · 2018-04-11T11:34:55Z

+            yield 'Repository does not exist!'
+            return
+
+        if (self.TEAMS[self.GH_ORG_NAME + ' maintainers'].is_member(user) == False):


Line is longer than allowed. (84 > 80)

Origin: LineLengthBear, Section: all.linelength.

gitmate-bot · 2018-04-11T11:34:55Z

+        new_issue_title = old_issue.title
+        new_issue_description = old_issue.description.rstrip()
+        issue_author = old_issue.author.username
+        extra_msg = '\n\nMigrated issue originally opened by @' + issue_author + \


Line is longer than allowed. (82 > 80)

Origin: LineLengthBear, Section: all.linelength.

gitmate-bot · 2018-04-11T11:34:56Z

+
+        for comment in old_issue.comments:
+            comm_text = comment.body.rstrip()
+            comm_url = url1.format(orig_host, org, repo_name_orig, issue_number) + \


Line is longer than allowed. (84 > 80)

Origin: LineLengthBear, Section: all.linelength.

gitmate-bot · 2018-04-11T11:34:57Z

+            comm_url = url1.format(orig_host, org, repo_name_orig, issue_number) + \
+                '#issuecomment-' + str(comment.number)
+            new_body = comm_text + '\n\nOriginally written by @' + \
+                comment.author.username + ' on ' + str(comment.updated) + ' UTC' + \


Line is longer than allowed. (84 > 80)

Origin: LineLengthBear, Section: all.linelength.

gitmate-bot · 2018-04-11T11:34:58Z

+        url2 = 'https://{}.com/{}/{}/issues/{}'.format(
+            final_host, org, repo_name_final, new_issue.number)
+
+        migrated_comment = 'Issue has been migrated to another [repository](' + \


Line is longer than allowed. (81 > 80)

Origin: LineLengthBear, Section: all.linelength.

aabhaas-vaish · 2018-04-11T12:36:56Z

Requested changes made, @meetmangukiya

jayvdb · 2018-04-15T12:34:46Z


        self.mock_org = create_autospec(github3.orgs.Organization)
        self.mock_gh = create_autospec(github3.GitHub)
+


unrelated change.

jayvdb · 2018-04-15T12:35:04Z

+        mock_maint_team.is_member.return_value = False
+
+        labhub.TEAMS = {
+                        'coala maintainers': mock_maint_team,


indent only 4 spaces

jayvdb · 2018-04-15T12:35:47Z

+        # No issue exists
+        mock_maint_team.is_member.return_value = True
+        self.mock_repo.get_issue = Mock(side_effect=RuntimeError('Error message',404))
+        testbot.assertCommand(cmd.format('coala', 'a', '21','coala','b'),


missing whitespace between args

jayvdb · 2018-04-15T12:36:24Z

+                              'has been closed already')
+        # Migrate issue
+        mock_maint_team.is_member.return_value = True
+        mock_iss = create_autospec(IGitt.GitHub.GitHub.GitHubIssue)


use mock_issue instead of shorted name

aabhaas-vaish · 2018-04-15T16:43:50Z

Requested changes made, @jayvdb

jayvdb · 2018-04-16T07:58:01Z

+        # Migrate issue
+        mock_maint_team.is_member.return_value = True
+        mock_issue = create_autospec(IGitt.GitHub.GitHub.GitHubIssue)
+        issue2 = create_autospec(IGitt.GitHub.GitHub.GitHubIssue)


why is one called mock_issue and the other issue2? that is confusing.

jayvdb · 2018-04-16T09:17:17Z

+        try:
+            assert org == self.GH_ORG_NAME or org == self.GL_ORG_NAME
+        except AssertionError:
+            yield 'First repository not owned by our org.'


First & second -> source & target

jayvdb · 2018-04-16T09:17:37Z

+        org2 = match.group(6)
+        repo_name_final = match.group(7)
+
+        try:


replace try with if

jayvdb · 2018-04-16T09:19:14Z

+            yield 'Issue cannot be migrated as it has been closed already.'
+            return
+
+        url1 = 'https://{}.com/{}/{}/issues/{}'


this isnt a url .. it is a format string.

confusing because here url1 is a format string, but below url2 is a real complete url.

jayvdb · 2018-04-16T09:19:42Z

+        new_issue_title = old_issue.title
+        new_issue_description = old_issue.description.rstrip() + '\n\n'
+        issue_author = old_issue.author.username
+        ext_msg = 'Migrated issue originally opened by @' + issue_author + \


pls dont use line continuations (\)

jayvdb · 2018-04-16T09:21:12Z

+            else:
+                raise RuntimeError(sterr, errno)
+
+        if str(old_issue.state) == 'closed':


it is safer to check that it is open.
then new states wont surprise everyone.

jayvdb · 2018-04-16T09:23:07Z

+        try:
+            assert org2 == self.GH_ORG_NAME or org2 == self.GL_ORG_NAME
+        except AssertionError:
+            yield 'Second repository not owned by our org.'


If there is only one message, you can use return 'foo' instead of yield & return

aabhaas-vaish · 2018-04-17T10:19:37Z

Changes made @jayvdb

jayvdb · 2018-04-17T11:39:41Z

+        org = match.group(2)
+        repo_name_orig = match.group(3)
+        issue_number = match.group(4)
+        user = msg.frm.nick


it is a bit strange seeing this variable mixed in amoung all the regex matches.

please read every line of your diff and ask yourself if it can be improved.

aabhaas-vaish · 2018-04-17T16:29:08Z

Made changes as requested, @jayvdb

meetmangukiya · 2018-04-22T04:12:24Z

                 )
        yield reply
+
+    @re_botcmd(pattern=r'^migrate\s+https://(github|gitlab)\.com/([^/]+)/([^/]+)/+issues/(\d+)\s+https://(github|gitlab)\.com/([^/]+)/([^/]+)/*$',  # Ignore LineLengthBear, PyCodeStyleBear


the regex won't work for gitlab sub groups, see regex for other commands to see how its done

Since this regex is most likely be duplicated in every command, make a new constant with the regex to match github/gitlab url, and then append it to your command. Do this as part of another issue after this is complete 😉

@meetmangukiya , I used the same regex as used in unassign_cmd and it uses the same regex. How should I go about this? 😕

What about we only capture the URL here, without using regex to parse it, and then split it with a helper function that can be re-used in each command which takes a git hoster URL ?

meetmangukiya · 2018-04-22T04:15:29Z

+        if target_repo not in self.REPOS:
+            return 'Target repository does not exist.'
+
+        # Ignore LineLengthBear, PyCodeStyleBear


meetmangukiya · 2018-04-22T04:19:52Z

+        source_url = 'https://{}.com/{}/{}/issues/{}'.format(
+            source_host, source_org, source_repo, issue_number)
+
+        # Ignore LineLengthBear, PyCodeStyleBear


Remove this and make the string multiline, something like

a = ('some long string which doesnt fit in 80 chars' 'Next part of the string. This both will be concantenated into one')

meetmangukiya · 2018-04-22T04:20:53Z

+            source_issue.title, target_issue_desc)
+        target_issue.labels = source_labels
+
+        # Ignore LineLengthBear, PyCodeStyleBear


Same as above

meetmangukiya · 2018-04-22T04:22:00Z

+        comment_ext = '\n\nOriginally commented by @{} on {} UTC and can be seen [here]({})'
+
+        for comment in source_issue.comments:
+            comm_url = source_url + '#issuecomment-' + str(comment.number)


I don't think this is the case for gitlab comments, use IGitt API to get comment link, if it doesn't exist, make upstream contribution

Will be excluding the comment URLs for the current PR and will make another PR when upstream contribution in IGitt is made.

meetmangukiya · 2018-04-22T04:22:45Z

+        target_issue.labels = source_labels
+
+        # Ignore LineLengthBear, PyCodeStyleBear
+        comment_ext = '\n\nOriginally commented by @{} on {} UTC and can be seen [here]({})'


Are you sure the time returned has no UTC offset?

I tested it on it on my own forks and there was no UTC offset included there. However, I wasn't able to find documentation explicitly mentioning anything about offsets 😅

aabhaas-vaish · 2018-04-23T17:13:37Z

Will do @meetmangukiya

nvzard · 2018-05-15T17:15:07Z

@random-access7 can you please make the requested changes?

aabhaas-vaish · 2018-05-16T05:56:35Z

@nvzard, will do by tonight.

migrate_issue plugin that adds ability to migrate an issue from a source repo to target repo, both owned by the org. Issue title, issue description and all comments are copied with a few additional details appended to the description and comments. Source issue is referenced in the target issue and closed after the migration is completed. Closes coala#518

meetmangukiya · 2018-06-12T14:34:49Z

@gitmate-bot rebase

gitmate-bot · 2018-06-12T14:34:51Z

Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently ⚠️

gitmate-bot · 2018-06-12T14:35:49Z

Automated rebase failed! Please rebase your pull request manually via the command line.

Reason:

Command: git rebase base/master

Exit Code: 128

Cause:

error: Failed to merge in the changes.

meetmangukiya · 2018-06-12T14:38:21Z

@random-access7 can you please resolve the merge conflicts? And we will make sure it merges before anything else so it doesn't lead to more conflicts

gitmate-bot added process/pending review size/S cool and awesome difficulty/medium labels Apr 7, 2018

gitmate-bot assigned aabhaas-vaish Apr 7, 2018

aabhaas-vaish force-pushed the master branch from 0ef7838 to 31ecd7a Compare April 7, 2018 16:09

meetmangukiya requested changes Apr 7, 2018

View reviewed changes

aabhaas-vaish force-pushed the master branch from 31ecd7a to aff0b9e Compare April 11, 2018 11:34

gitmate-bot reviewed Apr 11, 2018

View reviewed changes

aabhaas-vaish force-pushed the master branch from aff0b9e to 149accc Compare April 11, 2018 12:28

aabhaas-vaish force-pushed the master branch 4 times, most recently from f03dd02 to 6b8f154 Compare April 13, 2018 15:25

jayvdb requested changes Apr 15, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Apr 15, 2018

aabhaas-vaish force-pushed the master branch from 6b8f154 to 19168d0 Compare April 15, 2018 16:26

gitmate-bot added process/pending review and removed process/wip labels Apr 15, 2018

aabhaas-vaish force-pushed the master branch from 19168d0 to 69fdfdc Compare April 15, 2018 16:29

jayvdb reviewed Apr 16, 2018

View reviewed changes

jayvdb requested changes Apr 16, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels Apr 16, 2018

aabhaas-vaish force-pushed the master branch from 69fdfdc to e4528f1 Compare April 17, 2018 10:05

gitmate-bot added process/pending review and removed process/wip labels Apr 17, 2018

aabhaas-vaish force-pushed the master branch from e4528f1 to df21338 Compare April 17, 2018 10:31

jayvdb reviewed Apr 17, 2018

View reviewed changes

aabhaas-vaish force-pushed the master branch from df21338 to c74f2fe Compare April 17, 2018 16:20

meetmangukiya requested changes Apr 22, 2018

View reviewed changes

jayvdb added process/wip and removed process/pending review labels May 2, 2018

aabhaas-vaish force-pushed the master branch from c74f2fe to f631ff3 Compare May 16, 2018 19:58

aabhaas-vaish force-pushed the master branch from f631ff3 to 5e517e3 Compare May 16, 2018 20:01


		self.mock_org = create_autospec(github3.orgs.Organization)
		self.mock_gh = create_autospec(github3.GitHub)

Uh oh!

Conversation

aabhaas-vaish commented Apr 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewers Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabhaas-vaish Apr 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabhaas-vaish commented Apr 11, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabhaas-vaish commented Apr 15, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabhaas-vaish commented Apr 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aabhaas-vaish commented Apr 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

aabhaas-vaish commented Apr 7, 2018 •

edited

Loading

aabhaas-vaish Apr 7, 2018 •

edited

Loading

aabhaas-vaish commented Apr 17, 2018 •

edited

Loading