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

Augmented ErrorReporter API with informational messages #1192

Merged
merged 12 commits into from
May 25, 2022

Conversation

zekailin00
Copy link
Collaborator

@zekailin00 zekailin00 commented May 24, 2022

Fixes issue #1113. Added reportInfo to ErrorReporter.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Looks good! Congrats on your first PR! 🚀

I only had nitpicks. Once you've addressed those, I think we're good to go!

org.lflang/src/org/lflang/ErrorReporter.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/ErrorReporter.java Outdated Show resolved Hide resolved
org.lflang/src/org/lflang/ErrorReporter.java Show resolved Hide resolved
org.lflang/src/org/lflang/ErrorReporter.java Outdated Show resolved Hide resolved
@@ -94,17 +94,18 @@ private String report(String message, Severity severity, EObject obj) {
* (within the Eclipse IDE), then this also adds a marker to the editor.
*
* @param message The error message.
* @param severity One of IMarker.SEVERITY_ERROR or IMarker.SEVERITY_WARNING
* @param severity One of IMarker.SEVERITY_ERROR or IMarker.SEVERITY_WARNING or
Copy link
Member

Choose a reason for hiding this comment

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

...?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added severity info to the comment.

@@ -173,6 +174,16 @@ public String reportWarning(String message) {
return report(message, Severity.WARNING, null, null);
}

/**
* Report an info.
Copy link
Member

Choose a reason for hiding this comment

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

Since this is an override, you could just use:

/**
 * {@inheritdoc}
 */

(Or leave out the comment entirely, because I think JavaDoc is smart enough to inherit docs from the interface or super class automatically).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

comment is removed.

@@ -115,6 +115,20 @@ public String reportWarning(String message) {
return message;
}

/**
* Report the given message as a warning on the object currently under
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect comment. Just leave it out because it is an override, anyway?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

comment is removed.

@lhstrh lhstrh changed the title added info report when no main reactor Augmented ErrorReporter API with informational messages May 24, 2022
Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Almost there!

@@ -195,6 +201,17 @@ public String reportWarning(EObject obj, String message) {
return report(message, Severity.WARNING, obj);
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

This Javadoc comment can also be removed.

@@ -221,6 +238,19 @@ public String reportWarning(Path file, Integer line, String message) {
return report(message, Severity.WARNING, line, file);
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

Same

@@ -125,6 +132,16 @@ public String reportWarning(EObject object, String message) {
return message;
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

No Javadoc necessary.

@@ -146,6 +163,18 @@ public String reportWarning(Path file, Integer line, String message) {
return fullMessage;
}

/**
Copy link
Member

Choose a reason for hiding this comment

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

No Javadoc necessary.

@zekailin00
Copy link
Collaborator Author

repetitive comments are removed.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

🚀 🚀 🚀

@lhstrh lhstrh merged commit 0dcebcf into master May 25, 2022
@lhstrh lhstrh deleted the 1113-report-info-when-no-main branch May 25, 2022 04:59
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