-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: create and list issues #693
base: main
Are you sure you want to change the base?
Conversation
3b65c3b
to
f1dcac3
Compare
f1dcac3
to
c6f7a74
Compare
defbb83
to
3bff297
Compare
@kavitha186, we need unit tests to cover issue, list-issues, and list commands. Can you add those? I've started down this road with checkout.ts. Seems a bit bulky and in need of rework but thought I'd share how I'm thinking about adding coverage for commands. |
c6f7a74
to
dd8e026
Compare
@kavitha186, I've landed test coverage for all the existing commands. You should be good to go to do the same for the new commands introduced in this PR. |
f816316
to
2101b63
Compare
4fa9474
to
614b5e7
Compare
614b5e7
to
f328e68
Compare
@kavitha186, let’s add a screenshot of unit tests coverage. A recording demoing the functionality would be helpful too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tagging me for feedback, I really appreciate the work you all have done here. I really hope this is valuable for you too.
Something I'd really like is to be able to customize the description based on the repo.
For example, say there are 5 issues I'm scanning repos for. If those were autofixable I'd fix them via PR, but they aren't, hence an issue.
So I want to check out the repos, run some scripts, and generate the issue description.
In my example, say I identify that the repo only has 3 of the 5 issues. The issue that I'd file might have some instructions for how to address those 3 issues, and even ideally be able to show the permalink to the lines in question:
shepherd/examples/eslintrc-yml/shepherd.yml
Lines 3 to 7 in b1239dd
adapter: | |
type: github | |
search_query: repo:NerdWalletOSS/shepherd-demo path:/ filename:.eslintrc | |
hooks: | |
should_migrate: |
https://github.com/NerdWalletOSS/shepherd/blob/b1239dd92255d68fd00927fc54d42ea756128d05/examples/eslintrc-yml/shepherd.yml#L3-L7
In order to piece that URL together I'd also need some information. I could probably get the sha by calling git log myself, and I can get the file path / line numbers. Not sure if I'd have the org/repo name though.
@@ -82,6 +82,12 @@ hooks: | |||
post_checkout: npm install | |||
apply: mv .eslintrc .eslintrc.json | |||
pr_message: echo 'Hey! This PR renames `.eslintrc` to `.eslintrc.json`' | |||
issues: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does it mean to have this in the yml as well as the migration for PRs. Would it open a PR and an Issue? Are they mutually exclusive? (I'd kinda expect them to be mutually exclusive). Maybe update the description text above for what this does now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes they are mutually exclusive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you see the list of commands in the ReadMe section, it has description for each command.
pr
: Creates a PR for each repo with the message generated from thepr_message
hook.version
: Prints Shepherd versionissue
: Create, update, or close issues across multiple repository.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TheSavior, we're exposing issues as a top-level command in Shepherd (i.e., shepherd issues ...
). As it is currently implemented, it is mutually exclusive from other commands.
That said, I'm wondering if we should be thinking of incorporating hooks into issues. For example, we can have a should_create_issue
and post_create_issue
hooks analogous to should_migrate
and post_checkout
hooks.
08bc366
to
5d0fb3a
Compare
5d0fb3a
to
aeb2189
Compare
@TheSavior and @kavitha186, thank you for your continued feedback and contributions. While I think we've diverged a bit from the requirements Eli is after, I think Kavitha's foundational work is valuable and potentially adjustable to include Eli's use case. Use Case Current State If Solution 1 (Recommended)
Pros: No code change required to Shepherd Solutions 2
Pros: End user does not need to persist and manage state on disk Solution 3
Pros:
Cons:
|
@kavitha186, while I did some research on existing tooling to ensure we're not overlapping with feature sets already available elsewhere, I think it would be valuable to double check from your end. For example, does Just curious if there is anything we might have missed. |
I see there is a gh cli command to post issue or create PRs. Perhaps that is available only to manage a single repo as far as I see. |
Option 1 seems fine tbh, happy to persist things to dish. It seems like there is some information I couldn't easily get from just a user script running in should_migrate though. For example, in order to form those URLs, I'd need to know the org/repo name. I could probably shell out to git remote and figure it out, would that be your recommendation? I would also probably need that to key the data stored on disk to the right repo so that when the issue hook runs later I know where to look for that persisted repo info. |
Oh or maybe your point about the gh cli is that I should just use shepherd to search and filter the repos, and once I get to should migrate and run the custom script, that script should just call the gh cli. That would probably work too! |
My comment is with respect to adding the issues command to Shepherd. That said, I think calling |
859aa60
to
b104d76
Compare
b104d76
to
d436773
Compare
@kavitha186, can we address the CI failures? |
@nwalters512, would love your input and feedback on this PR when you get a chance. |
2d3bb05
to
9dfd6de
Compare
9dfd6de
to
91b7662
Compare
This PR is for the feature to post issues on the repositories with shepherd. if you are looking to post issues across multiple repositories, you can use the command "issue" to create, update , list posted issues and close issue
issue with the title, issue message, labels , state and state_reason provided in the migration scripts.
This enhancement feature is for the issue #87 and #590
Screenshots
Create Issue
Update Issue
Closing Issue
List Issues
Screencasts
Screen-2024-03-17-185925.mp4