GitCommitBear: Refactor issue regexes #2459

virresh · 2018-04-30T18:44:15Z

Increased modularity by delegating the task of selecting issue
regex to another function

For short term contributors: we understand that getting your commits well
defined like we require is a hard task and takes some learning. If you
look to help without wanting to contribute long term there's no need
for you to learn this. Just drop us a message and we'll take care of brushing
up your stuff for merge!

Checklist

I read the commit guidelines and I've followed
them.
I ran coala over my code locally. (All commits have to pass
individually. It is not sufficient to have "fixup commits" on your PR,
our bot will still report the issues for the previous commit.) You will
likely receive a lot of bot comments and build failures if coala does not
pass on every single commit!

After you submit your pull request, DO NOT click the 'Update Branch' button.
When asked for a rebase, consult coala.io/rebase
instead.

Please consider helping us by reviewing other peoples pull requests as well:

pick up any PR at https://coala.io/review
review it (check https://coala.io/reviewing for more info)
if you are sure that it needs work, use corobo mark wip <URL> to get it out
of the review queue.

The more you review, the more your score will grow at coala.io and we will
review your PRs faster!

jayvdb · 2018-05-01T03:27:26Z

@gitmate-bot rebase

gitmate-bot · 2018-05-01T03:27:29Z

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-05-01T03:29:48Z

Automated rebase with GitMate.io was successful! 🎉

jayvdb · 2018-05-01T03:35:05Z

bears/vcs/git/GitCommitBear.py

-            result_info = 'issue'
-            issue_ref_regex = r'(?:\w+/\w+)?#(\S+)'
+        self.initialise_issue_ref_regex(body_close_issue_full_url,
+                                        result_message,


ewww, you have a return value being put in result_message

jayvdb · 2018-05-01T03:51:09Z

bears/vcs/git/GitCommitBear.py

+            The current repository's host
+        """
+        if body_close_issue_full_url:
+            self.ISSUE_TYPE = 'full issue'


these are instance variables, not class variables like the issue requested.

you should have one class variable which is a regex with named groups which tells you everything you need to know.

Issue type can be derived from the result match data.

The class variable will need to be like SUPPORTED_HOST_KEYWORD_REGEX, being a dictionary with an entry for each hoster.

I think the scope of this patch needs to be "GitCommitBear: Add BitBucket support" , which would dovetail with HgCommit bear which would add Hg-on-BitBucket support.

This is roughly #1134

Does this mean that we have regex entries for each host and each type ?
Like

{ 'github' : { 'issue' : r(some regex), 'full issue' : r(some regex) }, 'some other hoster' :{ ... } }

jayvdb · 2018-05-01T05:26:04Z

bears/vcs/git/GitCommitBear.py

+                r'https?://{}\S+/issues/(\S+)'.format(re.escape(host)))
+        else:
+            self.ISSUE_TYPE = 'issue'
+            self.ISSUE_REF_REGEX = r'(?:\w+/\w+)?#(\S+)'


The difference between this and 'hg' URLs, is not actually related to hg at all.

These URLs, and the differences, are a property of the hoster, not the vcs software being used.

see #2310 (review)

virresh · 2018-05-03T02:38:22Z

Also once Git Commit bear supports bitbucket, we need to have an extra check around if body_close_fullissue_url to ensure that full issue is indeed supported by the host (since bitbucket doesn't support that)

jayvdb

Getting very close.

jayvdb · 2018-05-03T08:53:04Z

bears/vcs/git/GitCommitBear.py

@@ -25,6 +25,16 @@ class GitCommitBear(GlobalBear):
    LICENSE = 'AGPL-3.0'
    ASCIINEMA_URL = 'https://asciinema.org/a/e146c9739ojhr8396wedsvf0d'
    CAN_DETECT = {'Formatting'}
+    ISSUE_INFO = {
+        'github': {
+                    'issue': r'(?:\w+/\w+)?#(\S+)',


'issue' should be indented four spaces from 'github', i.e. appearing under hub..

jayvdb · 2018-05-03T08:53:12Z

bears/vcs/git/GitCommitBear.py

+    ISSUE_INFO = {
+        'github': {
+                    'issue': r'(?:\w+/\w+)?#(\S+)',
+                    'full issue': r'https?://github\S+/issues/(\S+)'


trailing comma

jayvdb · 2018-05-03T08:53:41Z

bears/vcs/git/GitCommitBear.py

+        'github': {
+                    'issue': r'(?:\w+/\w+)?#(\S+)',
+                    'full issue': r'https?://github\S+/issues/(\S+)'
+                  },


closing bracket should be at the same level as 'github'

jayvdb · 2018-05-03T08:54:01Z

bears/vcs/git/GitCommitBear.py

+        'gitlab': {
+                    'issue': r'(?:\w+/\w+)?#(\S+)',
+                    'full issue': r'https?://gitlab\S+/issues/(\S+)'
+                  }


add trailing commas (everywhere)

Should I put them in the SUPPORTED_HOST_KEYWORD_REGEX as well ?
(or that be left for a newcomer issue ?)

jayvdb · 2018-05-03T08:55:50Z

bears/vcs/git/GitCommitBear.py

            return

+        if body_close_issue_full_url:
+            self.ISSUE_INFO['type'] = 'full issue'


dont modify the class constant self.ISSUE_INFO.

It stops being a constant.

use an instance variable.

jayvdb · 2018-05-04T15:41:06Z

bears/vcs/git/GitCommitBear.py

            return

+        if body_close_issue_full_url:
+            self.issueType = 'full issue'


issueType -> issue_type . pep8 compliant naming

jayvdb · 2018-05-04T15:44:28Z

bears/vcs/git/GitCommitBear.py

-            result_info = 'full issue'
-            issue_ref_regex = (
-                r'https?://{}\S+/issues/(\S+)'.format(re.escape(host)))
+            result_message = ('Body of HEAD commit does not contain any '


if you keep {} ' at the end of this line, it wont be modified, making your diff much smaller.

Also you dont need to change either of the result_message = ... statements.

You can just add below

result_message = result_message.format(self.issue_type)

jayvdb · 2018-05-04T15:47:46Z

bears/vcs/git/GitCommitBear.py

@@ -359,7 +369,8 @@ def check_issue_reference(self, body,
                reference = compiled_issue_ref_regex.fullmatch(issue)
                if not reference:
                    yield Result(self, 'Invalid {} reference: '
-                                       '{}'.format(result_info, issue))
+                                       '{}'.format(
+                                        self.issueType, issue))


either put .format on this line, or intend this line another 3 spaces. pycodestyle isnt perfect - it will accept a lot of non-pep8 compliant indentations because it is quite a difficult problem to know the correct indentation level needed. But the fact you have a one space indentation here should be a clue that it is wrong even if pycodestyle accepts it.

Increased modularity by delegating the task of selecting issue regex to a new class variable Closes coala#2396

jayvdb · 2018-05-04T23:56:31Z

ack 97e9890

jayvdb · 2018-05-04T23:56:44Z

@gitmate-bot ff

gitmate-bot · 2018-05-04T23:56:45Z

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

gitmate-bot · 2018-05-04T23:56:58Z

Automated fastforward with GitMate.io was successful! 🎉

gitmate-bot added process/pending review size/XS area/genericbears difficulty/low labels Apr 30, 2018

gitmate-bot assigned virresh Apr 30, 2018

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

virresh force-pushed the RegexRefactor branch from 5824079 to 443525b Compare May 1, 2018 03:18

gitmate-bot added process/pending review and removed process/wip labels May 1, 2018

gitmate-bot force-pushed the RegexRefactor branch from 443525b to d76bd43 Compare May 1, 2018 03:29

jayvdb mentioned this pull request May 1, 2018

HgCommitBear.py: Add Mercurial support for commits #2310

Merged

2 tasks

jayvdb requested changes May 1, 2018

View reviewed changes

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

virresh force-pushed the RegexRefactor branch from d76bd43 to 8d0c9b4 Compare May 3, 2018 02:07

gitmate-bot added process/pending review and removed process/wip labels May 3, 2018

jayvdb requested changes May 3, 2018

View reviewed changes

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

virresh force-pushed the RegexRefactor branch 2 times, most recently from 76164f0 to 9566efc Compare May 4, 2018 12:51

gitmate-bot added process/pending review and removed process/wip labels May 4, 2018

jayvdb requested changes May 4, 2018

View reviewed changes

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

GitCommitBear: Refactor issue regexes

97e9890

Increased modularity by delegating the task of selecting issue regex to a new class variable Closes coala#2396

virresh force-pushed the RegexRefactor branch from 9566efc to 97e9890 Compare May 4, 2018 16:23

gitmate-bot added process/pending review and removed process/wip labels May 4, 2018

jayvdb approved these changes May 4, 2018

View reviewed changes

gitmate-bot added process/approved and removed process/pending review labels May 4, 2018

gitmate-bot removed the process/approved label May 4, 2018

gitmate-bot merged commit 97e9890 into coala:master May 4, 2018

jayvdb mentioned this pull request May 5, 2018

Update YAPF #2464

Closed

gitmate-bot added the process/approved label May 5, 2018

gitmate-bot removed the process/approved label Oct 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GitCommitBear: Refactor issue regexes #2459

GitCommitBear: Refactor issue regexes #2459

virresh commented Apr 30, 2018

jayvdb commented May 1, 2018

gitmate-bot commented May 1, 2018

gitmate-bot commented May 1, 2018

jayvdb May 1, 2018

jayvdb May 1, 2018

jayvdb May 1, 2018

virresh May 1, 2018

jayvdb May 1, 2018

virresh commented May 3, 2018

jayvdb left a comment

jayvdb May 3, 2018

jayvdb May 3, 2018

jayvdb May 3, 2018

jayvdb May 3, 2018

virresh May 3, 2018

jayvdb May 3, 2018

jayvdb May 4, 2018

jayvdb May 4, 2018

jayvdb May 4, 2018

jayvdb commented May 4, 2018

jayvdb commented May 4, 2018

gitmate-bot commented May 4, 2018

gitmate-bot commented May 4, 2018

GitCommitBear: Refactor issue regexes #2459

GitCommitBear: Refactor issue regexes #2459

Conversation

virresh commented Apr 30, 2018

Checklist

jayvdb commented May 1, 2018

gitmate-bot commented May 1, 2018

gitmate-bot commented May 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virresh commented May 3, 2018

jayvdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jayvdb commented May 4, 2018

jayvdb commented May 4, 2018

gitmate-bot commented May 4, 2018

gitmate-bot commented May 4, 2018