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

Add ability to put ticket num in either the PR title or the git branch name #44

Merged
merged 2 commits into from
Oct 4, 2018

Conversation

neilkimmett
Copy link
Contributor

We label our branches with the JIRA issue rather than the PR, so I added it as an option. I haven't managed to test this with our repo yet, but hopefully it'll work 🤞

@macklinu
Copy link
Owner

macklinu commented Oct 3, 2018

This is a cool idea! Thanks for contributing. Let me take a deeper look at this tonight/tomorrow and follow up. 😄

@neilkimmett
Copy link
Contributor Author

Not sure why CI failed? Looks like it actually passed?!
image

@neilkimmett
Copy link
Contributor Author

Testing this out on our repo and it appears that danger.git.branch isn't a thing. Investigating what to do instead.

@macklinu
Copy link
Owner

macklinu commented Oct 3, 2018

Check out these docs for the danger object API reference. Can’t remember off hand how to get the branch but hopefully this points you in the right direction https://danger.systems/js/reference.html#the-danger-dsl-root-objects

@macklinu
Copy link
Owner

macklinu commented Oct 3, 2018

also check out the danger pr command https://danger.systems/js/guides/the_dangerfile.html#using-danger-pr haven’t used it in a while but hopefully that helps in testing too :)

@neilkimmett
Copy link
Contributor Author

Shockingly theres actually nothing in there to get the branch 😱. Gonna take a look at either adding it to Danger, or pulling in https://github.com/jonschlinkert/git-branch in the plugin. Thoughts?

@macklinu
Copy link
Owner

macklinu commented Oct 3, 2018

what about the value of danger.github.pr.base? that might be the branch that is used to open the PR

@neilkimmett
Copy link
Contributor Author

neilkimmett commented Oct 3, 2018

danger.github.pr.head.ref seems to do the trick! Cheers pal.

- location can be either the PR title or the git branch name
* The location of the JIRA issue, either the PR title, or the git branch
* Defaults to `title`
*/
location?: "title" | "branch";
Copy link
Owner

Choose a reason for hiding this comment

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

I'm sort of wondering if we should even make this an option. Part of me wants to make this plugin "smarter" or closer to "zero config" and just look in the following places for a JIRA ticket ID without requiring the user to configure this:

  • danger.github.pr.title - PR title
  • danger.github.pr.head.ref - the PR head branch name
  • (I think that's it right now)

What do you think about that as someone who uses this plugin? Do you want more configurability, or do you want things to Just Work™?

Copy link
Owner

Choose a reason for hiding this comment

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

Maybe also check all the commit messages too? Link all the things 🔗

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh that would be pretty nice actually. What would it do if it found multiple tickets; just spit out links to all of em?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In that vein we could remove the need to specify a JIRA key and look for anything that looks like ABC[DEF]-123? In the meantime how would you feel about merging this and tackling all of that as a follow up?

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah this can be a follow up in a future release. This is a great non-breaking addition to the plugin. 🎉

@macklinu macklinu merged commit 96e6261 into macklinu:master Oct 4, 2018
@neilkimmett
Copy link
Contributor Author

Awesome, thanks dude. Any chance you could push a new version to NPM?

@macklinu
Copy link
Owner

macklinu commented Oct 4, 2018

Yeah lemme look at that today. I have semantic-release set up to auto-deploy, but I think my npm publishing token got revoked in the last few months, so I'll get a version published later today.

@neilkimmett
Copy link
Contributor Author

🙏 ty ty

@macklinu
Copy link
Owner

macklinu commented Oct 4, 2018

🎉 This PR is included in version 1.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

mcgrathg pushed a commit to mcgrathg/danger-plugin-jira-integration that referenced this pull request Apr 13, 2023
# 1.0.0 (2023-04-13)

### Bug Fixes

* **jiraIssue:** properly resolve URL ([af099b9](af099b9))
* **node:** support Node 4 ([d89d651](d89d651))
* only add one example JIRA key to warning ([macklinu#47](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/47)) ([56dee38](56dee38))

### Features

* `key` is now optional field ([2acddae](2acddae))
* **$options:** Adds message format function to options ([macklinu#33](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/33)) ([ecfead9](ecfead9))
* **jiraIssue:** add initial implementation ([#1](#1)) ([8638e2f](8638e2f))
* **options:** add location to options ([macklinu#44](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/44)) ([96e6261](96e6261))
* support multiple JIRA issues in PR title ([86c01d0](86c01d0)), closes [macklinu#8](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/8)
* support multiple JIRA projects ([macklinu#26](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/26)) ([f0a084f](f0a084f)), closes [macklinu#24](https://github.com/mcgrathg/danger-plugin-jira-integration/issues/24)

### BREAKING CHANGES

* **jiraIssue:** this commit introduces functionality ready for a 1.0.0 release.
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