GitCommitBear: Support BitBucket's issue reference #2466

virresh · 2018-05-05T14:34:04Z

Add support for BitBucket as a hoster and support it's limited issue
referencing style.

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

The commit shortlog doesnt describe the patch changes very well.

jayvdb · 2018-05-06T00:44:32Z

bears/vcs/git/GitCommitBear.py

@@ -34,14 +34,21 @@ class GitCommitBear(GlobalBear):
            'issue': r'(?:\w+/\w+)?#(\S+)',
            'full issue': r'https?://gitlab\S+/issues/(\S+)',
        },
+        'bitbucket': {
+            'issue': r'(?:\w+/\w+)?#(\S+)',


I think the (?:\w+/\w+)? here is not supported by bitbucket??

I think bitbucket allows an issue to be referred to using
close #11
or some word in between like close issue #11 or close bug #11 or close ticket #11
So I am not sure how to go about that, I thought the above regex is for scenarios where it matches a word before issue number (possibly a link to the repo), so maybe for bitbucket we can use it to serve to support these words ?

Please correct me, I am very bad at regex.

virresh · 2018-05-06T06:39:08Z

I have removed the leading link match, although it doesn't support the close issue #11 kind of style in bitbucket,
I tried adding the matching r' ?(?:bug|issue|ticket)? to SUPPORTED_HOST_KEYWORD, but inside the check_issue_reference it always ends up picking issue as the issue reference instead of #11 in case of closes and resolves, but it works for fixes ( example )
And making further non-capture groups breaks the logic for regexes below

virresh · 2018-05-06T06:48:37Z

Used an ugly hack:
The SUPPORTED_HOST_KEYWORD_REGEX is no longer a proper regex for bitbucket, instead it needs to be wrapped in a enclosing non-capturing clause.

This will work in the current arrangement since we replace SUPPORTED_HOST_KEYWORD_REGEX using a .format() into a template such as (?:{0})

ksdme · 2018-05-06T08:55:24Z

bears/vcs/git/GitCommitBear.py

+        'bitbucket': (r'[Cc]los(?:e[sd]?|ing)'
+                      r'|[Rr]esolv(?:e[sd]?|ing)'
+                      r'|[Ff]ix(?:e[sd]|ing)?'
+                      r')(?: ?(?:bug|issue|ticket)?'),


Replace the whitespace with \s, this is because any kind of whitespace is supported during regex joining.

The whitespace shouldn't be optional either if something is to proceed after the reference type. The regex should rather group the space and descriptor and make it optional (https://regex101.com/r/gJ9oKe/1/).

Thanks @ksdme

But I am not sure if we want to match with a newline or carriage return (since a \n or \r do not result in closing of the issue

Though yes thanks for pointing this out, multiple spaces and tabs do need to be accomodated 😅

Updated @ksdme
Please review :)

ksdme · 2018-05-06T11:21:23Z

bears/vcs/git/GitCommitBear.py

+        'bitbucket': (r'[Cc]los(?:e[sd]?|ing)'
+                      r'|[Rr]esolv(?:e[sd]?|ing)'
+                      r'|[Ff]ix(?:e[sd]|ing)?'
+                      r')(?:(?:[ \t]*(?:bug|issue|ticket)?)?'),


I am not sure about the [ \t]*, I think you should match the whitespace characters for one or more occurrences (https://regex101.com/r/cBwJ02/1).

I tried it for this and this
And no whitespace still works for closing the issue,

So should I go ahead and check that for one or more spaces then ?

If you match for zero or more occurrences of [ \t] then you would essentially be depending upon https://github.com/virresh/coala-bears/blob/4ff7382a06f48267a290fba41196866052b92226/bears/vcs/git/GitCommitBear.py#L38 to find the issue with the the reference. I don't think this should be the case.

So if I get it correct,
we should not allow fixesbug#11, but we should allow fixes bug#11 ?

Use https://confluence.atlassian.com/bitbucket/resolve-issues-automatically-when-users-push-code-221451126.html as your guide.

Yep
I read the guidelines there, but they don't seem to emphasize on the whitespaces,
As per the guidelines even ~~fixesbug#11~~, fixesbug #11, fixes bug#11, fixes bug #11, all three are valid.

So I accomodated all varieties of whitespaces. Not sure if I should limit it to only fixes bug #11

Also made a test repo, just to be sure of the effect of whitespaces 😅

Although the suggested guidelines do not explicitly state anything about whitespaces it does include a space while defining the format as <command> <issue id> and advertise this behavior in all the examples, also for the case of readability we should take it as an implicit guideline.

Oh
Cool
Thanks @ksdme

Opened https://bitbucket.org/site/master/issues/16240/issue-referencing-on-bitbucket-cloud

And off to support fixes bug#11 and fixes bug #11

ksdme · 2018-05-06T11:29:51Z

bears/vcs/git/GitCommitBear.py

+        'bitbucket': (r'[Cc]los(?:e[sd]?|ing)'
+                      r'|[Rr]esolv(?:e[sd]?|ing)'
+                      r'|[Ff]ix(?:e[sd]|ing)?'
+                      r')(?:(?:[ \t]*(?:bug|issue|ticket)?)?'),


The shortlog should say 'Support bitbucket type issue references' or something along those lines since you are specifically aiming to improve those aspects of the bear. The commit message could also be improved by removing the example.

jayvdb

bitbucket->BitBucket

jayvdb · 2018-05-06T15:06:55Z

tests/vcs/git/GitCommitBearTest.py

+                        'Another line, blablablablablabla.\n'
+                        'Closesticket #1112')
+        self.assertEqual(self.run_uut(
+                             body_close_issue=True), [])


this should be an error, yes?

This works as per guidelines 😅

I do not think anyone would have a reason to write it in such a skewed way. Maybe we should have an option for this bear indicating whether to stick to core guidelines or readability guidelines.

ksdme · 2018-05-07T12:17:18Z

bears/vcs/git/GitCommitBear.py

@@ -34,14 +34,22 @@ class GitCommitBear(GlobalBear):
            'issue': r'(?:\w+/\w+)?#(\S+)',
            'full issue': r'https?://gitlab\S+/issues/(\S+)',
        },
+        'bitbucket': {
+            'issue': r'#(\S+)',


https://confluence.atlassian.com/sourcetreekb/link-to-bitbucket-issue-tracker-from-commits-296911608.html seems to indicates that the regex required to match an issue would rather be #(\d+).

ksdme

I think it is very clear from https://confluence.atlassian.com/bitbucket/mark-up-comments-issues-and-commit-messages-321859781.html and https://confluence.atlassian.com/bitbucket/resolve-issues-automatically-when-users-push-code-221451126.html that we should only support the syntax <command> <issue id> irrespective of what works during commit actions. The command should also maintain readability and thus we should only support references written as fixes bug #11.

ksdme · 2018-05-07T12:42:38Z

bears/vcs/git/GitCommitBear.py

+        'bitbucket': (r'(?:(?:[Cc]los(?:e[sd]?|ing)'
+                      r'|[Rr]esolv(?:e[sd]?|ing)'
+                      r'|[Ff]ix(?:e[sd]|ing))'
+                      r'(?:\s+(?:(?:bug|issue|ticket)[ \t]*)?))'),


The \s+ in r'(?:\s+(?:(?:bug|issue|ticket)[ \t]*)?))'), should rather be [ \t]+ as an individual reference needs to be a single line.

ksdme · 2018-05-07T12:46:04Z

bears/vcs/git/GitCommitBear.py

    }
    SUPPORTED_HOST_KEYWORD_REGEX = {
-        'github': (r'[Cc]lose[sd]?'
+        'github': (r'(?:[Cc]lose[sd]?'


I do not think changes made to the regex of hosts other than BitBucket are either necessary or in the scope of this issue.

Oh
Okay
Did that to support fixes bug#166 but not fixesbug #11
Will revert 😅

Thanks :D

ksdme · 2018-05-07T12:51:39Z

bears/vcs/git/GitCommitBear.py

@@ -337,7 +350,7 @@ def check_issue_reference(self, body,

        concat_regex = '|'.join(kw for kw in self.CONCATENATION_KEYWORDS)
        compiled_joint_regex = re.compile(
-            r'(?:{0})\s+'           # match issue related keywords,
+            r'(?:{0})'              # match issue related keywords,


This change should be reverted too.

virresh · 2018-05-07T14:37:36Z

Supported only the sane version of <command> <issue id>

Please review @ksdme :)

ksdme

Everything looks good to me other than,

shortlog -> Support BitBucket type issue references &
hoster -> host in commit message.

virresh · 2018-05-07T15:15:09Z

Thanks @ksdme
The shortlog was exceeding character limit, so replaced support with add 😅

ksdme · 2018-05-07T18:00:53Z

tests/vcs/git/GitCommitBearTest.py

@@ -298,16 +298,140 @@ def test_check_issue_reference(self):

        # Adding BitBucket remote for testing
        self.run_git_command('remote', 'add', 'test',
-                             'https://bitbucket.com/user/repo.git')
+                             'https://user@bitbucket.org/user/gittest.git')


Is there a specific reason why the test repo url needs to be changed to this?

Oh
I did that to keep the url format consistent with the one suggested by bitbucket on the clone option for a repository

Although the above also redirects fine

Will change that 😅

ksdme · 2018-05-07T18:35:50Z

tests/vcs/git/GitCommitBearTest.py

@@ -300,14 +300,138 @@ def test_check_issue_reference(self):
        self.run_git_command('remote', 'add', 'test',
                             'https://bitbucket.com/user/repo.git')

-        # Unsupported Host - Bitbucket
+        # Unsupported Host and Issue reference combination - Bitbucket


The host is now supported.

ksdme · 2018-05-07T19:41:18Z

tests/vcs/git/GitCommitBearTest.py

+        self.run_git_command('remote', 'set-url', 'test',
+                             'git@bitbucket.org:user/repo.git')
+
+        # Unsupported Host and Issue reference combination - Bitbucket


You left out this instance of the comment.

Add support for BitBucket as a host and support it's limited issue referencing style. Closes coala#1134

jayvdb · 2018-05-07T23:24:05Z

ack 33cd9e4

jayvdb · 2018-05-07T23:24:20Z

@gitmate-bot ff

gitmate-bot · 2018-05-07T23:24:21Z

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-07T23:24:33Z

Automated fastforward with GitMate.io was successful! 🎉

gitmate-bot added process/pending review size/S area/newsettings difficulty/low importance/low labels May 5, 2018

gitmate-bot assigned virresh May 5, 2018

jayvdb requested changes May 6, 2018

View reviewed changes

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

virresh force-pushed the virresh/bitbucket branch from 290c993 to de28f74 Compare May 6, 2018 06:00

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

virresh force-pushed the virresh/bitbucket branch from de28f74 to c3be0a4 Compare May 6, 2018 06:46

ksdme suggested changes May 6, 2018

View reviewed changes

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

virresh force-pushed the virresh/bitbucket branch from c3be0a4 to 4ff7382 Compare May 6, 2018 10:52

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

ksdme suggested changes May 6, 2018

View reviewed changes

virresh force-pushed the virresh/bitbucket branch from 4ff7382 to 0a100a9 Compare May 6, 2018 11:43

virresh changed the title ~~GitCommitBear: Mandate Valid Issue reference~~ GitCommitBear: Support bitbucket's issue reference May 6, 2018

jayvdb requested changes May 6, 2018

View reviewed changes

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

jayvdb changed the title ~~GitCommitBear: Support bitbucket's issue reference~~ GitCommitBear: Support BitBucket's issue reference May 6, 2018

virresh force-pushed the virresh/bitbucket branch from 0a100a9 to 53f163c Compare May 7, 2018 11:04

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

virresh force-pushed the virresh/bitbucket branch from 53f163c to aa90c3f Compare May 7, 2018 11:05

ksdme reviewed May 7, 2018

View reviewed changes

ksdme suggested changes May 7, 2018

View reviewed changes

virresh force-pushed the virresh/bitbucket branch 2 times, most recently from 5507287 to 5b240a8 Compare May 7, 2018 14:05

ksdme suggested changes May 7, 2018

View reviewed changes

virresh force-pushed the virresh/bitbucket branch from 5b240a8 to 9e838a1 Compare May 7, 2018 15:14

ksdme reviewed May 7, 2018

View reviewed changes

virresh force-pushed the virresh/bitbucket branch from 9e838a1 to a130a27 Compare May 7, 2018 18:13

ksdme suggested changes May 7, 2018

View reviewed changes

virresh force-pushed the virresh/bitbucket branch from a130a27 to ad1f173 Compare May 7, 2018 18:39

ksdme suggested changes May 7, 2018

View reviewed changes

GitCommitBear: Add BitBucket type issue references

33cd9e4

Add support for BitBucket as a host and support it's limited issue referencing style. Closes coala#1134

virresh force-pushed the virresh/bitbucket branch from ad1f173 to 33cd9e4 Compare May 7, 2018 19:44

ksdme approved these changes May 7, 2018

View reviewed changes

jayvdb approved these changes May 7, 2018

View reviewed changes

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

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

gitmate-bot merged commit 33cd9e4 into coala:master May 7, 2018

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

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

GitCommitBear: Support BitBucket's issue reference #2466

GitCommitBear: Support BitBucket's issue reference #2466

Conversation

virresh commented May 5, 2018 • edited

Checklist

jayvdb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virresh May 6, 2018 • edited

Choose a reason for hiding this comment

virresh commented May 6, 2018 • edited

virresh commented May 6, 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

ksdme May 6, 2018 • edited

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 May 6, 2018 • edited

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

ksdme May 6, 2018 • edited

Choose a reason for hiding this comment

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

ksdme May 6, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksdme left a comment • edited

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 7, 2018

ksdme left a comment

Choose a reason for hiding this comment

virresh commented May 7, 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

jayvdb commented May 7, 2018

jayvdb commented May 7, 2018

gitmate-bot commented May 7, 2018

gitmate-bot commented May 7, 2018

virresh commented May 5, 2018 •

edited

virresh May 6, 2018 •

edited

virresh commented May 6, 2018 •

edited

ksdme May 6, 2018 •

edited

virresh May 6, 2018 •

edited

ksdme May 6, 2018 •

edited

ksdme May 6, 2018 •

edited

ksdme left a comment •

edited