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.

Sign up for GitHub

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

Jump to bottom

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
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bab38b8
feat($options): Adds location to options
neilkimmett Aug 28, 2018
63359a6
Merge branch 'master' into master
macklinu Oct 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ jiraIssue({
format(emoji, jiraUrls) {
// Optional Formatter
return "Some Custom Message";
}
},
location: "title" // Optional location, either 'title' or 'branch'
});
```

Expand Down
13 changes: 13 additions & 0 deletions src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,17 @@ describe("jiraIssue()", () => {
':link: JIRA Tickets: <a href="https://jira.net/browse/ABC-123">ABC-123</a>, <a href="https://jira.net/browse/DEF-456">DEF-456</a>'
);
});
it("supports JIRA key in the git branch", () => {
global.danger = {
github: { pr: { head: { ref: "ABC-808/some-things" } } }
};
jiraIssue({
key: "ABC",
location: "branch",
url: "https://jira.net/browse"
});
expect(global.message).toHaveBeenCalledWith(
':link: <a href="https://jira.net/browse/ABC-808">ABC-808</a>'
);
});
});
32 changes: 29 additions & 3 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ export interface Options {
* @returns {string}
*/
format?: (emoji: string, jiraUrls: string[]) => string;
/**
* 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. 🎉

}

const link = (href: string, text: string): string =>
Expand All @@ -41,7 +46,8 @@ const ensureUrlEndsWithSlash = (url: string) => {
* to include the JIRA issue identifier in the pull request title.
*/
export default function jiraIssue(options: Options) {
const { key = "", url = "", emoji = ":link:" } = options || {};
const { key = "", url = "", emoji = ":link:", location = "title" } =
options || {};
if (!url) {
throw Error(`'url' missing - must supply JIRA installation URL`);
}
Expand All @@ -56,8 +62,26 @@ export default function jiraIssue(options: Options) {
let match;
const jiraIssues = [];
// tslint:disable-next-line:no-conditional-assignment
while ((match = jiraKeyRegex.exec(danger.github.pr.title)) != null) {
let jiraLocation;
switch (location) {
case "title": {
jiraLocation = danger.github.pr.title;
break;
}
case "branch": {
jiraLocation = danger.github.pr.head.ref;
break;
}
default: {
throw Error(
`Invalid value for 'location', must be either "title" or "branch"`
);
}
}
match = jiraKeyRegex.exec(jiraLocation);
while (match != null) {
jiraIssues.push(match[0]);
match = jiraKeyRegex.exec(jiraLocation);
}
if (jiraIssues.length > 0) {
const jiraUrls = jiraIssues.map(issue =>
Expand All @@ -71,6 +95,8 @@ export default function jiraIssue(options: Options) {
message(`${emoji} ${jiraUrls.join(", ")}`);
}
} else {
warn(`Please add the JIRA issue key to the PR title (e.g. ${key}-123)`);
warn(
`Please add the JIRA issue key to the PR ${location} (e.g. ${key}-123)`
);
}
}