Skip to content

Commit 28c9e76

Browse files
akshita31akshita31
committed
PR feedback
1 parent aed6f9a commit 28c9e76

File tree

4 files changed

+34
-25
lines changed

4 files changed

+34
-25
lines changed

‎package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@
646646
},
647647
{
648648
"command": "csharp.reportIssue",
649-
"title": "Upload bug report to github",
649+
"title": "Start authoring a new issue on GitHub",
650650
"category": "CSharp"
651651
}
652652
],

‎src/features/commands.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { EventStream } from '../EventStream';
1919
import { PlatformInformation } from '../platform';
2020
import CompositeDisposable from '../CompositeDisposable';
2121
import OptionProvider from '../observers/OptionProvider';
22-
import fileIssue from './fileIssue';
22+
import reportIssue from './reportIssue';
2323
import { execChildProcess } from '../common';
2424

2525
export default function registerCommands(server: OmniSharpServer, platformInfo: PlatformInformation, eventStream: EventStream, optionProvider: OptionProvider): CompositeDisposable {
@@ -48,7 +48,7 @@ export default function registerCommands(server: OmniSharpServer, platformInfo:
4848
// Register command for adapter executable command.
4949
disposable.add(vscode.commands.registerCommand('csharp.coreclrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream)));
5050
disposable.add(vscode.commands.registerCommand('csharp.clrAdapterExecutableCommand', async (args) => getAdapterExecutionCommand(platformInfo, eventStream)));
51-
disposable.add(vscode.commands.registerCommand('csharp.reportIssue', async () => fileIssue(vscode, eventStream, execChildProcess, platformInfo.isValidPlatformForMono())));
51+
disposable.add(vscode.commands.registerCommand('csharp.reportIssue', async () => reportIssue(vscode, eventStream, execChildProcess, platformInfo.isValidPlatformForMono())));
5252

5353
return new CompositeDisposable(disposable);
5454
}

‎src/features/fileIssue.ts renamed to ‎src/features/reportIssue.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { ReportIssue } from "../omnisharp/loggingEvents";
1111

1212
const issuesUrl = "https://github.com/OmniSharp/omnisharp-vscode/issues/new";
1313

14-
export default async function fileIssue(vscode: vscode, eventStream: EventStream, execChildProcess: (command: string, workingDirectory?: string) => Promise<string>, isValidPlatformForMono: boolean) {
14+
export default async function reportIssue(vscode: vscode, eventStream: EventStream, execChildProcess: (command: string, workingDirectory?: string) => Promise<string>, isValidPlatformForMono: boolean) {
1515
const dotnetInfo = await getDotnetInfo(execChildProcess);
1616
const monoInfo = await getMonoIfPlatformValid(execChildProcess, isValidPlatformForMono);
1717
let extensions = getInstalledExtensions(vscode);
@@ -76,14 +76,7 @@ ${tableHeader}\n${table};
7676

7777
async function getMonoIfPlatformValid(execChildProcess: (command: string, workingDirectory?: string) => Promise<string>, isValidPlatformForMono: boolean): Promise<string> {
7878
if (isValidPlatformForMono) {
79-
let monoInfo: string;
80-
try {
81-
monoInfo = await getMonoVersion(execChildProcess);
82-
}
83-
catch (error) {
84-
monoInfo = "A valid mono installation could not be found. Using the built-in mono"
85-
}
86-
79+
let monoInfo = await getMonoVersion(execChildProcess);
8780
return `<details><summary>Mono Information</summary>
8881
${monoInfo}</details>`;
8982
}
@@ -92,11 +85,27 @@ async function getMonoIfPlatformValid(execChildProcess: (command: string, workin
9285
}
9386

9487
async function getDotnetInfo(execChildProcess: (command: string, workingDirectory?: string) => Promise<string>): Promise<string> {
95-
return execChildProcess("dotnet --info", process.cwd());
88+
let dotnetInfo: string;
89+
try {
90+
dotnetInfo = await execChildProcess("dotnet --info", process.cwd());
91+
}
92+
catch (error) {
93+
dotnetInfo = "A valid dotnet installation could not be found.";
94+
}
95+
96+
return dotnetInfo;
9697
}
9798

9899
async function getMonoVersion(execChildProcess: (command: string, workingDirectory?: string) => Promise<string>): Promise<string> {
99-
return execChildProcess("mono --version", process.cwd());
100+
let monoInfo: string;
101+
try {
102+
monoInfo = await execChildProcess("mono --version", process.cwd());
103+
}
104+
catch (error) {
105+
monoInfo = "A valid mono installation could not be found. Using the built-in mono";
106+
}
107+
108+
return monoInfo;
100109
}
101110

102111
function getInstalledExtensions(vscode: vscode) {

‎test/unitTests/features/fileIssue.test.ts renamed to ‎test/unitTests/features/reportIssue.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { getFakeVsCode } from "../testAssets/Fakes";
7-
import fileIssue from "../../../src/features/fileIssue";
7+
import reportIssue from "../../../src/features/reportIssue";
88
import { EventStream } from "../../../src/EventStream";
99
import TestEventBus from "../testAssets/TestEventBus";
1010
import { expect } from "chai";
1111
import { ReportIssue } from "../../../src/omnisharp/loggingEvents";
1212
import { vscode } from "../../../src/vscodeAdapter";
1313

14-
suite("File Issue", () => {
14+
suite("reportIssue", () => {
1515
const vscodeVersion = "myVersion";
1616
const csharpExtVersion = "csharpExtVersion";
1717
const monoInfo = "myMonoInfo";
@@ -72,54 +72,54 @@ suite("File Issue", () => {
7272
});
7373

7474
test(`${ReportIssue.name} event is created`, async () => {
75-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
75+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
7676
let events = eventBus.getEvents();
7777
expect(events).to.have.length(1);
7878
expect(events[0].constructor.name).to.be.equal(`${ReportIssue.name}`);
7979
});
8080

8181
test("${ReportIssue.name} event is created with the omnisharp-vscode github repo issues url", async () => {
82-
await fileIssue(vscode, eventStream, execChildProcess, false);
82+
await reportIssue(vscode, eventStream, execChildProcess, false);
8383
expect(execCommands).to.not.contain("mono --version");
8484
let event = <ReportIssue>eventBus.getEvents()[0];
8585
expect(event.url).to.be.equal("https://github.com/OmniSharp/omnisharp-vscode/issues/new");
8686
});
8787

8888
test("The body contains the vscode version", async () => {
89-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
89+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
9090
let event = <ReportIssue>eventBus.getEvents()[0];
9191
expect(event.body).to.contain(encodeURIComponent(`**VSCode version**: ${vscodeVersion}`));
9292
});
9393

9494
test("The body contains the csharp extension version", async () => {
95-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
95+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
9696
let event = <ReportIssue>eventBus.getEvents()[0];
9797
expect(event.body).to.contain(encodeURIComponent(`**C# Extension**: ${csharpExtVersion}`));
9898
});
9999

100100
test("dotnet info is obtained and put into the body", async() => {
101-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
101+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
102102
expect(execCommands).to.contain("dotnet --info");
103103
let event = <ReportIssue>eventBus.getEvents()[0];
104104
expect(event.body).to.contain(dotnetInfo);
105105
});
106106

107107
test("mono information is obtained when it is a valid mono platform", async () => {
108-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
108+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
109109
expect(execCommands).to.contain("mono --version");
110110
let event = <ReportIssue>eventBus.getEvents()[0];
111111
expect(event.body).to.contain(monoInfo);
112112
});
113113

114114
test("mono information is not obtained when it is not a valid mono platform", async () => {
115-
await fileIssue(vscode, eventStream, execChildProcess, false);
115+
await reportIssue(vscode, eventStream, execChildProcess, false);
116116
expect(execCommands).to.not.contain("mono --version");
117117
let event = <ReportIssue>eventBus.getEvents()[0];
118118
expect(event.body).to.not.contain(monoInfo);
119119
});
120120

121121
test("The body contains all the name, publisher and version for the extensions that are not builtin", async () => {
122-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
122+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
123123
let event = <ReportIssue>eventBus.getEvents()[0];
124124
expect(event.body).to.contain(extension2.packageJSON.name);
125125
expect(event.body).to.contain(extension2.packageJSON.publisher);
@@ -130,7 +130,7 @@ suite("File Issue", () => {
130130
});
131131

132132
test("issuesUrl is put into the event url", async() => {
133-
await fileIssue(vscode, eventStream, execChildProcess, isValidForMono);
133+
await reportIssue(vscode, eventStream, execChildProcess, isValidForMono);
134134
let event = <ReportIssue>eventBus.getEvents()[0];
135135
expect(event.url).to.be.equal("https://github.com/OmniSharp/omnisharp-vscode/issues/new");
136136
});

0 commit comments

Comments
 (0)