Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix 'edit' actions documented arguments #826

Merged
merged 2 commits into from
Feb 2, 2025

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Jan 28, 2025

Docs only change.
Seems like arguments for pr.edit and pr.issue were backwards.


At some point the documentation for editing and issue and a pr drafted
from the code.

Update `get_repo_number_from_varargs` to reflect the documentation.
The documentation says `Octo pr [number] {repo}`

However, the code expected `Octo pr {repo} [number]`.
Fix this confusion by conforming to the documentation.

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

Checklist

  • Passing tests and linting standards
  • Documentation updates in README.md and doc/octo.txt

@wd60622
Copy link
Collaborator

wd60622 commented Jan 29, 2025

Should we make it behave the other way though? That is closer to the gh cli.

Thoughts?

@ldelossa
Copy link
Contributor Author

Oh! I don't feel strongly, if you'd like to see this, easy enough to make it happen :).

I don't mind at all to actually flip the code, and leave the docs the way they are today.

@wd60622
Copy link
Collaborator

wd60622 commented Jan 29, 2025

I think itd be good to be consistent with the cli

Thanks for making the change

At some point the documentation for editing and issue and a pr drafted
from the code.

Update `get_repo_number_from_varargs` to reflect the documentation.
The documentation says `Octo pr [number] {repo}`

However, the code expected `Octo pr {repo} [number]`.
Fix this confusion by conforming to the documentation.

Signed-off-by: ldelossa <[email protected]>
@ldelossa
Copy link
Contributor Author

@wd60622 handled your requested change.

@wd60622 wd60622 merged commit 781545b into pwntester:master Feb 2, 2025
3 checks passed
benelan added a commit to benelan/octo.nvim that referenced this pull request Feb 3, 2025
TheLeoP added a commit to TheLeoP/octo.nvim that referenced this pull request Feb 3, 2025
TheLeoP added a commit to TheLeoP/octo.nvim that referenced this pull request Feb 3, 2025
wd60622 pushed a commit that referenced this pull request Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants