pwntester / octo.nvim

Edit and review GitHub issues and pull requests from the comfort of your favorite editor
MIT License
2.42k stars 136 forks source link

Add `Octo pr create` command #178

Closed pwntester closed 3 years ago

pwntester commented 3 years ago

PR created with new Octo pr create :smile:

hahuang65 commented 3 years ago

Hmm, got an error when trying to create a PR:

gh: Head sha can't be blank, Base sha can't be blank, No commits between master and hh/test-pipeline-filters, Head ref must be a branch

I entered master for the base branch, and hh/test-pipeline-filters for the HEAD.

As you can see, there is a difference between them:

$ git log --oneline master..hh/test-pipeline-filters
1654c36b0 (HEAD -> hh/test-pipeline-filters) TEST: Add test for pipeline filters returning too much data

Also, could the title be pre-populated if there's only a single commit in the PR?

pwntester commented 3 years ago

Weird, Have you pushed the hh/test-pipeline-filters branch to origin? Maybe I should do that as part of the PR creation as gh does

hahuang65 commented 3 years ago

Yeah, that is what it was. It wasn't pushed to remote.

pwntester commented 3 years ago

Would you prefer octo to do that? It should be straigtforward to push the branch to origin remote before creating the PR

hahuang65 commented 3 years ago

I think gh has a decent workflow (at least for me personally). That is, if it doesn't have a remote branch, it'll offer to push it up, and let you name it. If it does have a remote branch, it assumes that's the remote branch.

pwntester commented 3 years ago

@hahuang65 Please try the latest version of the PR which:

hahuang65 commented 3 years ago

The autofilling of the title works slightly different than gh. gh will set the title to the first line of the commit messge, and comment to the rest of the git commit message.

Also, even if I had my branch on remote, it didn't pre-fill that for the HEAD branch.

Finally, when I answered Yes to create PR, it just kind of finished without any messages, and without actually creating the PR

pwntester commented 3 years ago

The autofilling of the title works slightly different than gh. gh will set the title to the first line of the commit messge, and comment to the rest of the git commit message.

Can you try again, should be fixed on 1209342

Also, even if I had my branch on remote, it didn't pre-fill that for the HEAD branch.

That should be fixed in 220013a

Finally, when I answered Yes to create PR, it just kind of finished without any messages, and without actually creating the PR

Can you print output and stderr in commands.lua:759 and let me know what is the anwser from the API?

hahuang65 commented 3 years ago

Those fixes look great... As for the API response... I'm not sure I'm too helpful. I don't create PRs that often this week so I didn't have a chance to test this more than once:

I made these changes:

          local choice = vim.fn.confirm("Create PR?", "&Yes\n&No\n&Cancel", 2)
          print("Choice is " .. choice)
          if choice == 1 then
            gh.run(
              {
                args = {"api", "graphql", "-f", string.format("query=%s", query)},
                cb = function(output, stderr)
                  print(output)
                  print(stderr)
                  if stderr and not utils.is_blank(stderr) then
                    vim.notify(stderr, 2)
                  elseif output then
                    local resp2 = vim.fn.json_decode(output)

Note the print(output) and print(stderr). I added those first. It didn't print anything.

Then I added the print("Choice is " .. choice) and that printed out Choice is 1 and also the output and stderr. I'm leaving that out for privacy reasons of my employer, but can separately message you if needed.

It seems as though the Choice is 1 maybe delayed something (since it asked me to press Return) and then the PR created successfully? Not too sure. Next time I make a PR, I will try again.

hahuang65 commented 3 years ago

Okay, I created another PR with the same flow as last time.

It created successfully... but for some reason, it also automatically requested a review from someone. In this case, it happened to be correct, but I'm wondering what may have caused it, and if there's a way to disable/modify the selection?

hahuang65 commented 3 years ago

Yeah this was successful again, but this time it didn't auto-assign the reviewer.

I think the PR is mostly good? The last couple outstanding questions I have are only related to the reviewers. 1) Why did that one get auto-assigned one time? 2) When searching for reviewers/assignees, how come some people don't show up in the search results, even if I have the name typed exactly?

pwntester commented 3 years ago

Sorry for the radio silence, been quite busy lately but hopefully will get more time to work on Octo next days.

Why did that one get auto-assigned one time?

That is not caused by Octo, probably there is an Action that auto assigns the PR on creation?

When searching for reviewers/assignees, how come some people don't show up in the search results, even if I have the name typed exactly?

Do these users have less than 10 repos? Can you please open a new issue for that?