Skip to content

Get Jira Issues action and Update PR description action#1

Open
dstotz wants to merge 5 commits into
masterfrom
get-jira-issues
Open

Get Jira Issues action and Update PR description action#1
dstotz wants to merge 5 commits into
masterfrom
get-jira-issues

Conversation

@dstotz

@dstotz dstotz commented Jun 19, 2024

Copy link
Copy Markdown
Contributor

Adds two new GitHub actions

  • get-jira-issues: obtains a list of Jira issues for the specified projects from the commit messages
  • update-pr-description: updates a specified set of content in the PR description for easy updates of content such as the list of jira issues on a release PR

@dstotz dstotz added the enhancement New feature or request label Jun 19, 2024
@dstotz dstotz requested a review from klismannsm June 19, 2024 05:02

@klismannsm klismannsm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Two general issues to address:

  1. node_modules was not ignored. Did you add them to speed up the CI process?

  2. The code is using semi-colons at the end of lines in almost every line, but some lines are not. We should choose one style and stick with it, possibly adding a prettier config file to handle this.

Comment thread lib/get-jira-issues.js Outdated
Comment thread lib/get-jira-issues.js Outdated
Comment thread lib/get-jira-issues.js Outdated
Comment thread lib/update-pr-description.js Outdated
Comment on lines +34 to +43
const matchRegex = new RegExp(matchRegexString, 'g');
let newDescription
console.log("Checking for matching content using regex:", matchRegex)
if (matchRegexString === undefined || !matchRegex.test(currentDescription)) {
console.log("No matching content found, adding new contents to the end")
newDescription = `${currentDescription}\n\n${newContents}`;
} else {
console.log("Matching content found, replacing it with new contents")
newDescription = currentDescription.replace(matchRegex, newContents);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

(improvement): This can be extracted into a separated method to improve readability.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@dstotz

dstotz commented Jul 8, 2024

Copy link
Copy Markdown
Contributor Author

Two general issues to address:

  1. node_modules was not ignored. Did you add them to speed up the CI process?
  2. The code is using semi-colons at the end of lines in almost every line, but some lines are not. We should choose one style and stick with it, possibly adding a prettier config file to handle this.
  1. You actually have to commit node_modules for custom JavaScript GitHub actions according to their docs. So that's why I included it https://docs.github.com/en/actions/creating-actions/creating-a-javascript-action#commit-tag-and-push-your-action-to-github If you know of a better/different way, I'm all for it!
  2. Good call, I'll set up prettier in this project and run through a full lint to fix those things.

I will address other code comments in line with GitHub reviews

@dstotz dstotz requested a review from klismannsm July 9, 2024 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants