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

Update the module with ballerina 0.990.0 #60

Merged
merged 7 commits into from Dec 8, 2018
Merged

Update the module with ballerina 0.990.0 #60

merged 7 commits into from Dec 8, 2018

Conversation

kalaiyarasiganeshalingam
Copy link
Contributor

@kalaiyarasiganeshalingam kalaiyarasiganeshalingam commented Dec 7, 2018

Purpose

Update the module with ballerina 0.990.0.

In the github connector, only 41 testcase are successfully run and other testcases are failure because the data map.

Goals

Describe the solutions that this feature/fix will introduce to resolve the problems described above

Approach

Describe how you are implementing the solutions. Include an animated GIF or screenshot if the change affects the UI (email documentation@wso2.com to review all UI text). Include a link to a Markdown file or Google doc if the feature write-up is too long to paste here.

User stories

Summary of user stories addressed by this change>

Release note

Brief description of the new feature or bug fix as it will appear in the release notes

Documentation

Link(s) to product documentation that addresses the changes of this PR. If no doc impact, enter “N/A” plus brief explanation of why there’s no doc impact

Training

Link to the PR for changes to the training content in https://github4.com/wso2/WSO2-Training, if applicable

Certification

Type “Sent” when you have provided new/updated certification questions, plus four answers for each question (correct answer highlighted in bold), based on this change. Certification questions/answers should be sent to certification@wso2.com and NOT pasted in this PR. If there is no impact on certification exams, type “N/A” and explain why.

Marketing

Link to drafts of marketing content that will describe and promote this feature, including product page changes, technical articles, blog posts, videos, etc., if applicable

Automation github4.tests

  • Unit github4.tests

    Code coverage information

  • Integration github4.tests

    Details about the test cases and coverage

Security checks

Samples

Provide high-level details about the samples related to this feature

Related PRs

List any other related PRs

Migrations (if applicable)

Describe migration steps and platforms on which migration has been tested

Test environment

List all JDK versions, operating systems, databases, and browser/versions on which this feature/fix was tested

Learning

Describe the research phase and any blog posts, patterns, libraries, or add-ons you used to solve the problem.

README.md Outdated
repository = rep;
} else {
io:println(err);

Copy link
Member

Choose a reason for hiding this comment

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

Please remove new line here.


// Public constants
# Pull request state open, closed and merged
@final public string PULL_REQUEST_STATE_ALL = "[\"OPEN\",\"CLOSED\",\"MERGED\"]";
final string PULL_REQUEST_STATE_ALL = "[\"OPEN\",\"CLOSED\",\"MERGED\"]";
Copy link
Member

@ldclakmal ldclakmal Dec 8, 2018

Choose a reason for hiding this comment

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

Removing public access modifier here block the usage of these variables for end users. Please correct it all the places where applicable.

Suggested change
final string PULL_REQUEST_STATE_ALL = "[\"OPEN\",\"CLOSED\",\"MERGED\"]";
public final string PULL_REQUEST_STATE_ALL = "[\"OPEN\",\"CLOSED\",\"MERGED\"]";

# Closed state
@final public string STATE_CLOSED = "[\"CLOSED\"]";
final string STATE_CLOSED = "[\"CLOSED\"]";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final string STATE_CLOSED = "[\"CLOSED\"]";
public final string STATE_CLOSED = "[\"CLOSED\"]";

# Merged state
@final public string STATE_MERGED = "[\"MERGED\"]";
final string STATE_MERGED = "[\"MERGED\"]";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final string STATE_MERGED = "[\"MERGED\"]";
public final string STATE_MERGED = "[\"MERGED\"]";

# Open state
@final public string STATE_OPEN = "[\"OPEN\"]";
final string STATE_OPEN = "[\"OPEN\"]";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final string STATE_OPEN = "[\"OPEN\"]";
public final string STATE_OPEN = "[\"OPEN\"]";

# State open and closed
@final public string STATE_ALL = "[\"OPEN\",\"CLOSED\"]";
final string STATE_ALL = "[\"OPEN\",\"CLOSED\"]";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
final string STATE_ALL = "[\"OPEN\",\"CLOSED\"]";
public final string STATE_ALL = "[\"OPEN\",\"CLOSED\"]";

target_repository.owner = {};
} else {
var result = RepositoryOwner.convert(source_json.owner);
if(result is RepositoryOwner) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(result is RepositoryOwner) {
if (result is RepositoryOwner) {

target_repository.primaryLanguage = {};
} else {
var result = Language.convert(source_json.primaryLanguage);
if(result is Language) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(result is Language) {
if (result is Language) {

# + githubGraphQlClient - HTTP client endpoint
public type Client client object {

public http:Client githubRestClient;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public http:Client githubRestClient;
http:Client githubRestClient;

public type Client client object {

public http:Client githubRestClient;
public http:Client githubGraphQlClient;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public http:Client githubGraphQlClient;
http:Client githubGraphQlClient;

# + labelList - List of labels for the issue
# + assigneeList - Users to be assigned to the issue
# + return - Created issue object or Connector error
remote function createIssue(string repositoryOwner, string repositoryName, string issueTitle, string issueContent,
Copy link
Member

@ldclakmal ldclakmal Dec 8, 2018

Choose a reason for hiding this comment

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

All of these remote functions should have public access modifier. Otherwise, this prevents the enduser to call these. Right?

}
}
} else {
error err = error(GITHUB_ERROR_CODE
Copy link
Member

Choose a reason for hiding this comment

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

Please format the code here.

remote function Client.getColumnListNextPage(ColumnList columnList) returns ColumnList|error {

if (columnList.hasNextPage()) {
var jsonQuery = stringToJson(columnList.columnListQuery);
Copy link
Member

Choose a reason for hiding this comment

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

Please format the code in everywhere of this remote function.

if (repository is Repository) {
repositoryOwner = repository.owner.login;
repositoryName = repository.name;
} else if (repository is (string, string)){
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
} else if (repository is (string, string)){
} else if (repository is (string, string)) {

//Check for empty payloads and errors
json|error validatedResponse = getValidatedResponse(response, GIT_ISSUES);

if(validatedResponse is json) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(validatedResponse is json) {
if (validatedResponse is json) {

if (result is Project) {
return result;
} else {
error err = error(GITHUB_ERROR_CODE
Copy link
Member

Choose a reason for hiding this comment

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

Please format the code here.

returns RepositoryList|error {

string organizationName = "";
if(organization is Organization) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(organization is Organization) {
if (organization is Organization) {


error err => {
if (response is http:Response) {
if(response.getJsonPayload() is error) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if(response.getJsonPayload() is error) {
if (response.getJsonPayload() is error) {

Fix

Made the changes in the testcases
@VijithaEkanayake-zz VijithaEkanayake-zz merged commit 3588e11 into ballerina-platform:master Dec 8, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants