User can assign him/herself to issue when pushing a new branch. #59

StevenXL · 2019-02-28T22:21:49Z

Resolves #15

vrom911

Nice start!

src/Hit/Cli.hs

src/Hit/Git.hs

src/Hit/Issue.hs

vrom911

Nice job! But I see one problem: we now get the issue info twice in the new command. I would like to avoid that and to reuse the issue info that is fetched in the new command anyway. That means that in attempAssigment you don't need to use issue' function.

src/Hit/Git.hs

src/Hit/Issue.hs

stack.yaml

StevenXL · 2019-03-06T01:23:01Z

@vrom911 Please let me know what you think. :-)

vrom911

@StevenXL , unfortunately, it doesn't work for me. Did you have a chance to test it somehow?

Also, I noticed that a bunch of old behaviour is broken now. From what I noticed the issues displayed in a different way know. I continue to test what else is broken now.

@chshersh, we need some tests asap, cause I'm sure this is because of the new version of the GitHub. The token is not working for me now, I don't know why.

chshersh · 2019-03-06T02:38:40Z

@vrom911 @StevenXL When I call new <n> -a command I see the following error:

ParseError "Error in $: key \"updated_at\" not present"

This doesn't work for me either, unfortunately 😞

Regarding tests: need to think how to make tests automatic. Maybe not for all commands. But should be possible. At least for issue command. Maybe we can create test repository only to test hit-on. But even without the repository it should be possible do test some commands...

StevenXL · 2019-03-06T18:37:25Z

Hm... I think this exposes a problem with the github library.

For example, if I run hit new 99, and there is no issue 99 in the repo, with github-0.20, I would get the following output:

  HTTPError (HttpExceptionRequest Request {
  host                 = "api.github.com"
  port                 = 443
  secure               = True
  requestHeaders       = [("Authorization","<REDACTED>"),("User-Agent","github.hs/0.7.4"),("Accept","application/vnd.github.preview")]
  path                 = "/repos/StevenXL/hit-on/issues/99"
  queryString          = ""
  method               = "GET"
  proxy                = Nothing
  rawBody              = False
  redirectCount        = 10
  responseTimeout      = ResponseTimeoutDefault
  requestVersion       = HTTP/1.1
}
 (StatusCodeException (Response {responseStatus = Status {statusCode = 404, statusMessage = "Not Found"}, responseVersion = HTTP/1.1, responseHeaders = [("Server","GitHub.com"),("Date","Wed, 06 Mar 2019 18:31:42 GMT"),("Content-Type","application/json; charset=utf-8"),("Transfer-Encoding","chunked"),("Status","404 Not Found"),("X-RateLimit-Limit","5000"),("X-RateLimit-Remaining","4999"),("X-RateLimit-Reset","1551900702"),("X-OAuth-Scopes","admin:business, admin:gpg_key, admin:org, admin:org_hook, admin:public_key, admin:repo_hook, delete_repo, gist, notifications, repo, user, write:discussion"),("X-Accepted-OAuth-Scopes","repo"),("X-GitHub-Media-Type","github.v3; param=preview"),("Access-Control-Expose-Headers","ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type"),("Access-Control-Allow-Origin","*"),("Strict-Transport-Security","max-age=31536000; includeSubdomains; preload"),("X-Frame-Options","deny"),("X-Content-Type-Options","nosniff"),("X-XSS-Protection","1; mode=block"),("Referrer-Policy","origin-when-cross-origin, strict-origin-when-cross-origin"),("Content-Security-Policy","default-src 'none'"),("Content-Encoding","gzip"),("X-GitHub-Request-Id","0A2F:403A:900CD5:11B977A:5C80120E")], responseBody = (), responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}) "{\"message\":\"Not Found\",\"documentation_url\":\"https://developer.github.com/v3/issues/#get-a-single-issue\"}"))

However, if I run the same command, but simply upgrade github package to 0.21, I get this output:

ParseError "Error in $: key \"updated_at\" not present"

So, my code is surely wrong, but the ParseError is misleading. I think we should be getting back an error tagged with the HTTPError data constructor.

I will keep looking, but let me know if you all have some thoughts.

vrom911 · 2019-03-07T01:52:42Z

@StevenXL, I don't think that your code is wrong. It looks like that the new GitHub library is broken, and the error it returns looks very much like JSON parsing error, which means that they broke some encoders/decoders in the new release. I think we could try to create an issue there with the minimal example of this behaviour. Do you want to do this? 🙂

StevenXL · 2019-03-22T17:09:27Z

@StevenXL, I don't think that your code is wrong. It looks like that the new GitHub library is broken, and the error it returns looks very much like JSON parsing error, which means that they broke some encoders/decoders in the new release. I think we could try to create an issue there with the minimal example of this behaviour. Do you want to do this? 🙂

Hello! I will try to provide a minimal, reproducible example on the github repository and then link to that here when I do.

StevenXL · 2019-03-22T18:16:45Z

@vrom911 I couldn't write a small enough reproduction. (I was trying to use the script interpreter feature of stack but couldn't figure out how to get it to use github-0.21 given that v0.21 is not in any resolver).

This is a link to the issue in the github package.

chshersh · 2019-06-26T02:58:42Z

@StevenXL I see that the issue is fixed (at least is says so) and github-0.22 is released. Can you try this with the newer github version?

StevenXL · 2019-06-26T15:58:50Z

Hi! I can work on this starting on June 6th. I’ll get back in touch then. On June 25, 2019 at 8:58:44 PM, Dmitrii Kovanikov (notifications@github.com) wrote: @StevenXL <https://github.com/StevenXL> I see that the issue is fixed (at least is says so) and github-0.22 is released. Can you try this with the newer github version? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#59?email_source=notifications&email_token=ABYMRSIDYI6J3ZYARQDY423P4LLOHA5CNFSM4G27OE2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYSFK4Q#issuecomment-505697650>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYMRSPZIHW2MN6NZMQQ4Z3P4LLOHANCNFSM4G27OE2A> .

StevenXL · 2019-06-26T16:00:27Z

Sorry - I meant July 6th.

…

On Wed, Jun 26, 2019 at 9:58 AM Steven Leiva ***@***.***> wrote: Hi! I can work on this starting on June 6th. I’ll get back in touch then. On June 25, 2019 at 8:58:44 PM, Dmitrii Kovanikov ( ***@***.***) wrote: @StevenXL <https://github.com/StevenXL> I see that the issue is fixed (at least is says so) and github-0.22 is released. Can you try this with the newer github version? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#59?email_source=notifications&email_token=ABYMRSIDYI6J3ZYARQDY423P4LLOHA5CNFSM4G27OE2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYSFK4Q#issuecomment-505697650>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABYMRSPZIHW2MN6NZMQQ4Z3P4LLOHANCNFSM4G27OE2A> .

-- Steven Leiva 305.528.6038 leiva.steven@gmail.com http://www.linkedin.com/in/stevenleiva

StevenXL · 2019-07-10T02:06:04Z

@chshersh Hi! I am trying to add github-0.22 to the project, but I am ending up in dependency hell (please excuse the phrasing). If you pull down this PR and run stack build, you will run into the following issue:

Error: While constructing the build plan, the following exceptions were encountered:

In the dependencies for transformers-compat-0.6.5:
    transformers-0.5.6.2 from stack configuration does not match >=0.3 && ==0.2.*
needed due to hit-on-0.1.0.0 -> transformers-compat-0.6.5

I don't know how to proceed from here. Any help would be greatly appreciated.

chshersh · 2019-07-10T05:17:52Z

@StevenXL I can start building the project using the following stack.yaml file:

resolver: lts-13.27

extra-deps:
  - relude-0.5.0
  - shellmet-0.0.1

  - github-0.22
  - binary-orphans-1.0.1  # for github
  - binary-instances-1    # for github
  - time-compat-1.9.2.2   # for github

StevenXL · 2019-07-11T19:07:18Z

@chshersh I was able to build with your suggestion. Thank you. Will continue to work on this.

StevenXL · 2019-07-11T20:52:52Z

@chshersh OK I think the functionality is finally working. Let me know your thoughts.

chshersh

I like progress 👍 And I have some suggestions

src/Hit/Issue.hs

src/Hit/Cli.hs

src/Hit/Issue.hs

src/Hit/Git.hs

chshersh

This looks great! I have one minor comment. Also, CI is failing because of HLint

src/Hit/Issue.hs

vrom911

Nice! I have a couple of questions/suggestion

vrom911 · 2019-07-13T06:32:09Z

hit-on.cabal

@@ -62,7 +62,7 @@ library

  build-depends:       base-noprelude ^>= 4.12.0.0
                     , ansi-terminal >= 0.8
-                     , github ^>= 0.20
+                     , github ^>= 0.22


We should carefully check if this version broke something else again...

src/Hit/Cli.hs

src/Hit/Issue.hs

vrom911 · 2019-07-13T06:39:51Z

src/Hit/Issue.hs

+assignToIssue issue = withOwnerRepo assignAction >>= \case
+    Left err              -> errorMessage (T.pack $ show err) >> exitFailure
+    Right AlreadyAssigned -> successMessage "Already assigned to issue"
+    Right NewlyAssigned   -> successMessage "Assigned to issue"


Let's add the issue number and name in here (ideally the link as well, so the user could just click and go there).

I think this is a great idea but would like that change to be an separate issue / pr.

chshersh

The code is okay. But I still see two problems with the implementation:

The issue number is displayed in a wrong way under the hit issue command

 ➤   [#IssueNumber 81] Implement `hit diff` command with pretty diff
 ➤   [#IssueNumber 80] Add `hit uncommit` command

I see the following error when trying to assign myself to an issue:

  HTTPError (HttpExceptionRequest Request {
  host                 = "api.github.com"
  port                 = 443
  secure               = True
  requestHeaders       = [("User-Agent","github.hs/0.21"),("Accept","application/vnd.github.v3+json"),("Authorization","<REDACTED>")]
  path                 = "/repos/kowainik/hit-on/issues/79"
  queryString          = ""
  method               = "PATCH"
  proxy                = Nothing
  rawBody              = False
  redirectCount        = 10
  responseTimeout      = ResponseTimeoutDefault
  requestVersion       = HTTP/1.1
}
 (StatusCodeException (Response {responseStatus = Status {statusCode = 500, statusMessage = "Internal Server Error"}, responseVersion = HTTP/1.1, responseHeaders = [("Date","Wed, 17 Jul 2019 05:09:02 GMT"),("Content-Type","application/json; charset=utf-8"),("Content-Length","0"),("Server","GitHub.com"),("Status","500 Internal Server Error"),("X-RateLimit-Limit","5000"),("X-RateLimit-Remaining","4994"),("X-RateLimit-Reset","1563343672"),("X-OAuth-Scopes","read:discussion, read:gpg_key, read:org, read:public_key, read:repo_hook, repo, user"),("X-Accepted-OAuth-Scopes",""),("X-GitHub-Media-Type","github.v3; format=json"),("Access-Control-Expose-Headers","ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type"),("Access-Control-Allow-Origin","*"),("Strict-Transport-Security","max-age=31536000; includeSubdomains; preload"),("X-Frame-Options","deny"),("X-Content-Type-Options","nosniff"),("X-XSS-Protection","1; mode=block"),("Referrer-Policy","origin-when-cross-origin, strict-origin-when-cross-origin"),("Content-Security-Policy","default-src 'none'"),("X-GitHub-Request-Id","AFC4:2B04:1FE60A:29B1E1:5D2EAD6E")], responseBody = (), responseCookieJar = CJ {expose = []}, responseClose' = ResponseClose}) ""))

My initial guess would be that this is because I don't have enough permissions for my token. But I checked it at GitHub and as you can see in this response, the token has repo permission. So I don't know why this is happening 🤔 It would be nice to specify, what permissions from the token are required for this to work.

piq9117 · 2019-08-07T19:25:42Z

it sounds like y’all are experiencing the same problem with the github package as we did. We think it has something to do with the encoding where some fields are optional but the package puts in a Nothing which then becomes null instead of just leaving out that field. I’m guessing the request will succeed with curl or some other method like postman?

chshersh · 2019-08-13T08:53:06Z

@piq9117 Parsing in github-0.21 was broken which stopped this PR from moving forward. The issue with parsing should be fixed in github-0.22 (as promised by CHANGELOG). I'm not sure what it's the problem now. This either might be wrong token scopes or something else. I agree with your proposed strategy for testing this: write curl command to test that the token is valid. And if curl command works, try to implement github version of it and open issue to the github library if it doesn't work.

vrom911 · 2019-09-12T09:37:25Z

Hey @StevenXL, do you have plans for this PR?
Sorry for bothering!

StevenXL · 2019-09-13T13:48:07Z

Hey @StevenXL, do you have plans for this PR?
Sorry for bothering!

@vrom911 It's no bother at all. I am going to close out the PR so if someone else wants to work on it, they don't feel the need to ask for "permission" or to check in on this PR.

Thank you for the reminder.

StevenXL mentioned this pull request Feb 28, 2019

When new branch for issue created add the user as assignee #15

Closed

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from 035599d to 32d4398 Compare February 28, 2019 22:24

vrom911 requested review from vrom911 and chshersh March 1, 2019 01:40

vrom911 assigned StevenXL Mar 1, 2019

vrom911 added enhancement New feature or request GitHub GitHub API CLI command-line interface labels Mar 1, 2019

vrom911 reviewed Mar 1, 2019

View reviewed changes

src/Hit/Cli.hs Outdated Show resolved Hide resolved

src/Hit/Git.hs Outdated Show resolved Hide resolved

src/Hit/Git.hs Outdated Show resolved Hide resolved

src/Hit/Issue.hs Outdated Show resolved Hide resolved

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from 32d4398 to fbf9635 Compare March 1, 2019 15:13

vrom911 reviewed Mar 4, 2019

View reviewed changes

src/Hit/Git.hs Outdated Show resolved Hide resolved

chshersh reviewed Mar 4, 2019

View reviewed changes

src/Hit/Issue.hs Outdated Show resolved Hide resolved

stack.yaml Outdated Show resolved Hide resolved

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch 3 times, most recently from da08c53 to a42950d Compare March 6, 2019 01:16

vrom911 reviewed Mar 6, 2019

View reviewed changes

chshersh mentioned this pull request Mar 16, 2019

Think on ways to test `GitHub-related functionality automatically #62

Open

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from a42950d to b9dcffe Compare July 10, 2019 02:00

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from b9dcffe to cba73c3 Compare July 10, 2019 02:16

Add github-0.22 as a dependency.

d522bed

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from cba73c3 to ec6e2b9 Compare July 11, 2019 19:58

User can add self to issue when creating branch.

77a0e80

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from ec6e2b9 to 77a0e80 Compare July 11, 2019 20:52

chshersh requested review from vrom911, chshersh and kahlil29 July 12, 2019 02:15

chshersh suggested changes Jul 12, 2019

View reviewed changes

StevenXL added 3 commits July 12, 2019 12:54

Remove "get issue" code duplication.

4827232

Adhere to style guide.

21aff7b

Remove unused getIssueTitle function.

f3cadbb

chshersh reviewed Jul 13, 2019

View reviewed changes

src/Hit/Issue.hs Show resolved Hide resolved

vrom911 reviewed Jul 13, 2019

View reviewed changes

Only assign to issue if user is not already assigned.

56e5200

StevenXL force-pushed the stevenxl/15-optionally-add-user-as-assignee branch from a2fe9f8 to 56e5200 Compare July 16, 2019 15:35

StevenXL added 2 commits July 16, 2019 15:48

Add documentation for the "assign" flag of the "new" command.

e90655b

Display "Already assigned" as an infoMessage.

5915fa2

chshersh suggested changes Jul 17, 2019

View reviewed changes

StevenXL closed this Sep 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User can assign him/herself to issue when pushing a new branch. #59

User can assign him/herself to issue when pushing a new branch. #59

StevenXL commented Feb 28, 2019 •

edited by chshersh

Loading

vrom911 left a comment

vrom911 left a comment

StevenXL commented Mar 6, 2019

vrom911 left a comment

chshersh commented Mar 6, 2019

StevenXL commented Mar 6, 2019

vrom911 commented Mar 7, 2019

StevenXL commented Mar 22, 2019

StevenXL commented Mar 22, 2019

chshersh commented Jun 26, 2019

StevenXL commented Jun 26, 2019 via email

StevenXL commented Jun 26, 2019 via email

StevenXL commented Jul 10, 2019 •

edited

Loading

chshersh commented Jul 10, 2019

StevenXL commented Jul 11, 2019

StevenXL commented Jul 11, 2019

chshersh left a comment

chshersh left a comment

vrom911 left a comment

vrom911 Jul 13, 2019

vrom911 Jul 13, 2019

StevenXL Jul 16, 2019

chshersh left a comment

piq9117 commented Aug 7, 2019

chshersh commented Aug 13, 2019

vrom911 commented Sep 12, 2019

StevenXL commented Sep 13, 2019

User can assign him/herself to issue when pushing a new branch. #59

User can assign him/herself to issue when pushing a new branch. #59

Conversation

StevenXL commented Feb 28, 2019 • edited by chshersh Loading

vrom911 left a comment

Choose a reason for hiding this comment

vrom911 left a comment

Choose a reason for hiding this comment

StevenXL commented Mar 6, 2019

vrom911 left a comment

Choose a reason for hiding this comment

chshersh commented Mar 6, 2019

StevenXL commented Mar 6, 2019

vrom911 commented Mar 7, 2019

StevenXL commented Mar 22, 2019

StevenXL commented Mar 22, 2019

chshersh commented Jun 26, 2019

StevenXL commented Jun 26, 2019 via email

StevenXL commented Jun 26, 2019 via email

StevenXL commented Jul 10, 2019 • edited Loading

chshersh commented Jul 10, 2019

StevenXL commented Jul 11, 2019

StevenXL commented Jul 11, 2019

chshersh left a comment

Choose a reason for hiding this comment

chshersh left a comment

Choose a reason for hiding this comment

vrom911 left a comment

Choose a reason for hiding this comment

vrom911 Jul 13, 2019

Choose a reason for hiding this comment

vrom911 Jul 13, 2019

Choose a reason for hiding this comment

StevenXL Jul 16, 2019

Choose a reason for hiding this comment

chshersh left a comment

Choose a reason for hiding this comment

piq9117 commented Aug 7, 2019

chshersh commented Aug 13, 2019

vrom911 commented Sep 12, 2019

StevenXL commented Sep 13, 2019

StevenXL commented Feb 28, 2019 •

edited by chshersh

Loading

StevenXL commented Jul 10, 2019 •

edited

Loading