search

ljay79 / jira-tools

Project Aid for Jira - Google Spreadsheet Add-on for Jira Integration
GNU General Public License v3.0
112 stars 46 forks source link

JiraUpdateTicket.gs #139

Open paul-lemon opened 5 years ago

paul-lemon commented 5 years ago

Name would be better as UpdateJiraTicketManager

Single Responsibility - Update a series of JIRA tickets using the same column / field defintions Use of a class will enable refactoring

  1. Reduce length and complexity of method updateJiraIssues (approx 70 lines)
  2. Remove need for convoluted error handling of field names identified in https://github.com/ljay79/jira-tools/pull/130 `if (responseData.errors != null) { Object.keys(responseData.errors).forEach(function (fieldid) { jiraErrorMessage = jiraErrorMessage + "{Field:" + fieldid + "}: " + responseData.errors[fieldid] + ", "; }); }

`

  1. Function below should be private function getMatchingJiraFields(allJiraFields, headerRow) {
ljay79 commented 5 years ago

We wont need to include the string "jira" everywhere :) as the entire code base is all about jira.

Ticket vs Issue I notice myself, mixing up the same thing calling it sometimes Ticket and other time Issue. Lets try (going forward) to stick to Atlassian terminology: Issue

So my name suggestion including above statements would be: IssueUpdateManager

paul-lemon commented 5 years ago

agreed - I think some naming conventions in a Contributing.MD to avoid future naming mishaps too.

ljay79 commented 5 years ago

Sure, as soon i have more than 1 convention :)

paul-lemon commented 5 years ago

packageRowForUpdate should be private also.

paul-lemon commented 5 years ago

This is underway with the re-factoring work I am doing. The file has been renamed but I aslo plan in moving a lot of the code into the Issues and IssueFields models and the updateIssues controller. I expect the file will be removed from the code base once that is completed.