Skip to content

feat: several Git2Gus improvements. See the details in description.#62

Merged
reiniergs merged 9 commits intoforcedotcom:masterfrom
jag-j:master
Apr 23, 2020
Merged

feat: several Git2Gus improvements. See the details in description.#62
reiniergs merged 9 commits intoforcedotcom:masterfrom
jag-j:master

Conversation

@jag-j
Copy link
Collaborator

@jag-j jag-j commented Apr 16, 2020

  1. Show github issue link in the description of the created GUS work item (W-7251050 (https://gus.my.salesforce.com/a07B00000087CpBIAU))
  2. Prepend a custom-text to the subject line of issues (W-7387696 (https://gus.my.salesforce.com/a07B0000008BGDZIA4))
  3. Do not post created GUS work item as a comment, but rather update the initial comment (W-7251060 (https://gus.my.salesforce.com/a07B00000087CpVIAU))
  4. Fix description formatting for newly created work items (W-7251029 (https://gus.my.salesforce.com/a07B00000087Co8IAE))
  5. Show GUS work item as a hyperlink.

url: 'github/git2gus-test/#30',
title: 'new issue',
body: 'some description',
body: '### some title\nsome description',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test to make sure we process markdown.

};

describe('createGusItem action', () => {
it('should call queue push with the right values', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Felt this was a redundant test. Changed it to make sure we process we strip down markdown to GUS formatting.

req,
body:
'Sorry we could wait until Heroku connect make the syncronization.'
'Sorry we could wait until Heroku connect make the synchronization.'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed a typo.

Copy link

@AnanyaJha AnanyaJha Apr 16, 2020

Choose a reason for hiding this comment

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

is this message saying we could not wait until heroku connect could make the syncronization? or that we had to wait until heroku connect could make the synchronization?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point! I should be "could not". @reiniergs - thoughts ?

} = req.body;
const { config } = req.git2gus;
const { hideWorkItemUrl } = config;
const { suppressGithubComments } = config;
Copy link
Collaborator Author

@jag-j jag-j Apr 16, 2020

Choose a reason for hiding this comment

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

This config to suppress commenting on Github issues, but rather update the description instead. Prevents constant messaging to external customers when they cannot act on those.

Choose a reason for hiding this comment

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

this is cool

};
function formatToGus(url, body) {
var formattedDescription;
remark().use(markdown).use(strip).process(body, (err, file) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Strip markdown to GUS format

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This show how this works and I tried with a few github issues from the past and seems to work as expected - http://remarkjs.github.io/strip-markdown/

if (err) {
throw err;
}
formattedDescription = 'Github issue link: '.concat(url, '\n', String(file));
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adds the github link at the top of the description. We can make it a table as @reiniergs suggested. But will involve more work and I can take that up after finishing up the changelist sync story I'm currently working.

});
expect(workItemUrl).toBe(
'https://gus.lightning.force.com/lightning/r/ADM_Work__c/qwerty1234/view'
'[W-234123](https://gus.lightning.force.com/lightning/r/ADM_Work__c/qwerty1234/view)'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a nice a have feature to show linked GUS item as a hyperlink instead of the full URL.

}

async function updateIssue(req, body, suppressGithubComments) {
if (suppressGithubComments) {
Copy link

Choose a reason for hiding this comment

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

Shouldn't this method run updateDescription or createComment depending on suppressGithubComments's value ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, that's the intention. It seems to work since suppressGithubComments config is boolean and not a string or is it a better practice to explicitly check the value?

issue: { labels, url, title, body, milestone }
issue: { labels, url, body, milestone }
} = req.body;
var {
Copy link
Contributor

Choose a reason for hiding this comment

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

use let here instead of var or I will rather compute a new variable like normalizedTitle and avoid doing mutation

return null;
}
};
function formatToGus(url, body) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please move this function into separate files

return formattedDescription;
}

async function updateIssue(req, body, suppressGithubComments) {
Copy link
Contributor

@reiniergs reiniergs Apr 16, 2020

Choose a reason for hiding this comment

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

move helper function into separate files


async function updateIssue(req, body, suppressGithubComments) {
if (suppressGithubComments) {
return await Github.updateDescription({
Copy link
Contributor

@reiniergs reiniergs Apr 16, 2020

Choose a reason for hiding this comment

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

if suppressGithubComments is true we shouldn't write the comments in the description of the issue, it will be hard to keep in sync/updated the description moving forward. I will say supressGithubComments suppress any communication back to Github

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let me bring this up with my PM who initially suggested this. FYI @ntotten @smaddox-sf

Copy link
Contributor

@reiniergs reiniergs left a comment

Choose a reason for hiding this comment

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

+1

@reiniergs reiniergs changed the title Several Git2Gus improvements. See the details in description. feat: several Git2Gus improvements. See the details in description. Apr 23, 2020
@reiniergs reiniergs merged commit 69c0aee into forcedotcom:master Apr 23, 2020
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.

4 participants