Skip to content

Add updating existing open issue with the same title if it exists#71

Merged
JasonEtco merged 25 commits intoJasonEtco:masterfrom
PrinsFrank:feature-update-existing-issue-when-present
Dec 6, 2020
Merged

Add updating existing open issue with the same title if it exists#71
JasonEtco merged 25 commits intoJasonEtco:masterfrom
PrinsFrank:feature-update-existing-issue-when-present

Conversation

@PrinsFrank
Copy link
Copy Markdown
Contributor

Hey @JasonEtco, I tried to implement #63 , but I'm a bit out of my depth here as I'm not familiar with developing actions. How can I test these changes without publishing my fork on the marketplace?

@PrinsFrank PrinsFrank changed the title WIP: Add updating existing open issue with the same title if it exists Add updating existing open issue with the same title if it exists Nov 7, 2020
Copy link
Copy Markdown
Owner

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome @PrinsFrank 😱 ❤️

I left a couple of notes for improvements. As for testing, you have two options - either writing unit tests here and seeing that they pass (which we need to do anyway), or using your forked action. It doesn't need to be published to Marketplace; you can just do:

uses: PrinsFrank/create-an-issue@feature-update-existing-issue-when-present

Comment thread index.js Outdated
Comment thread index.js
Comment thread index.js Outdated
Comment thread index.js
@PrinsFrank
Copy link
Copy Markdown
Contributor Author

I think I have everything running correctly, and will continue tomorrow with writing tests.

@PrinsFrank PrinsFrank requested a review from JasonEtco November 10, 2020 20:42
Copy link
Copy Markdown
Owner

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great @PrinsFrank ✨ The tests are perfect, the implementation is 🔥. Thank you for all your work on it so far!

I left a couple of very minor comments, the one in the search query is needed to make sure we're finding the right issue titles. Let me know if I can clarify anything! I think after these this should be good to go 🚀

Comment thread index.js Outdated
Comment thread index.js Outdated
Comment thread tests/index.test.js Outdated
Comment thread tests/index.test.js Outdated
Comment thread tests/index.test.js Outdated
Comment thread index.js Outdated
Comment thread README.md Outdated
@PrinsFrank PrinsFrank requested a review from JasonEtco November 12, 2020 19:19
@PrinsFrank
Copy link
Copy Markdown
Contributor Author

@JasonEtco, do you have any time to review this?

@Ameausoone
Copy link
Copy Markdown

Hello, I would be interested by this feature ! (Awesome work by the way).

I could implement #55 when this issue will be merged.

Copy link
Copy Markdown
Owner

@JasonEtco JasonEtco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me, sorry it took so long for me to come back to re-review it! Thanks for your work on this, especially the tests and docs 🙌

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.

3 participants