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.

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

LibComp 4 issues PR #38

Merged
merged 9 commits into from
Nov 2, 2020
Merged

LibComp 4 issues PR #38

merged 9 commits into from
Nov 2, 2020

Conversation

relhajj
Copy link
Collaborator

@relhajj relhajj commented Sep 5, 2020

  • close issue More meaningful presentation of metrics #31 : screenshot is below. I changed some minor things to make the units more clear without using up too much space. Also updated some of the descriptions of the metrics which are seen by hovering (cant show that in the screenshot but if you install the new package you can check them out)

image

@relhajj relhajj requested a review from snadi September 5, 2020 05:44
@snadi
Copy link
Contributor

snadi commented Sep 6, 2020

@relhajj For security & performance, can you change "(Percent)" to maybe "% of Issues"? Otherwise, it is not clear what percent this refers to

@snadi
Copy link
Contributor

snadi commented Sep 6, 2020

@relhajj i could not install the zip file because it says it is incompatible with my IDE. I'm running IntelliJ 20.20.1

returnValue = " " + in.readLine() + "\n" + " " + in.readLine();
}
}
if (typeofMaven == 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

what does typeOfMaven mean.. what is 1 vs 2? Can you perhaps create an enumeration for this for better readability like MVN_DEP_TYPE = {GRADLE, MAVEN} or something like that ?


String filePath = this.filePath +"\\"+ project_name +"domains.json";
File myFile = new File(filePath);
JSONObject Mainobj = null; // = new JSONObject();
Copy link
Contributor

Choose a reason for hiding this comment

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

please start variable names with a small letter mainObj .. probably mainJSONObj?

} catch (IOException e) {
e.printStackTrace();
}
JSONObject objM = new JSONObject(content);
Copy link
Contributor

Choose a reason for hiding this comment

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

can you try to use more descriptive variable names? what does objM mean?




public void EnabledDomain(int domain, String project_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this function name is unclear.. is it enableDomain.. is it isEnabledDomain? Don't think it's the latter since it doesn't return a bool but please use function names that indicate the action being taken or the check being performed etc

}


private int domainExist(int domain, JSONArray arr)
Copy link
Contributor

Choose a reason for hiding this comment

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

again the function name does not indicate the action/check being performed

return index;
}

public boolean isEnabled(int domain, String project_name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is what enabled?

TextAttributes attributes = EditorColorsManager.getInstance().getGlobalScheme().getAttributes(DebuggerColors.BREAKPOINT_ATTRIBUTES);
TextAttributes softerAttributes = attributes.clone();
boolean dependenciesExists = false;
int i = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is i?

TextAttributes attributes = EditorColorsManager.getInstance().getGlobalScheme().getAttributes(DebuggerColors.BREAKPOINT_ATTRIBUTES);
TextAttributes softerAttributes = attributes.clone();
boolean dependenciesExists = false;
int i = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

what is i

} // end of actionPErformed


public int detectDependenciesPSI(FileASTNode psinode)
Copy link
Contributor

Choose a reason for hiding this comment

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

so when it says Dependency or Dependencies, does that mean Gradle? I see functions for maven but not gradle.. Should you rename these so it's clear it's for gradle?

private String librariesURL = "https://smr.cs.ualberta.ca/comparelibraries/api/libraries/?format=json";
private String chartsURL = "https://smr.cs.ualberta.ca/comparelibraries/api/charts/?format=json";
private String metricsURL = "https://smr.cs.ualberta.ca/comparelibraries/api/metrics/?format=json&latestdate=";
private String updateUrllink = "https://smr.cs.ualberta.ca/comparelibraries/api/pluginusers/";
Copy link
Contributor

Choose a reason for hiding this comment

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

@relhajj these are still hard-coded domains, not in a config file. Given time, we can ignore it but this isn't what we agreed on in Issue #36

@snadi snadi merged commit 5d8d464 into master Nov 2, 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.

2 participants