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

Fixing different eclipse warnings (missing comments, unused parameters, static constants access) (#1486) #1495

Conversation

speckyspooky
Copy link
Contributor

@speckyspooky speckyspooky commented Nov 11, 2023

This change includes the fixing of different eclipse warnings:

  • missing comments
  • removed unused "Map param" from IPage-interface
  • replaced access to constants in a more static way (based on eclipse warnings/hints)

@speckyspooky speckyspooky added this to the 4.14 milestone Nov 11, 2023
@speckyspooky speckyspooky added the BugFix Change to correct issues label Nov 11, 2023
@speckyspooky speckyspooky self-assigned this Nov 11, 2023
@@ -195,10 +235,20 @@ public void newLine(boolean endParagraph) throws BirtException {
}
}

/**
* Get the fre space
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be done

import org.eclipse.birt.report.engine.layout.pdf.font.FontInfo;
import org.w3c.dom.css.CSSValue;

/**
* Class to implements the text style
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry, which typo?

Copy link
Contributor

Choose a reason for hiding this comment

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

remove the s from implements

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

public int getWordSpacing() {
return wordSpacing;
}

/**
* Check if text is underlined
Copy link
Contributor

Choose a reason for hiding this comment

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

the text

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be done

/**
* Check if text is underlined
*
* @return Return the check result
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be done

public boolean isLinethrough() {
return lineThrough;
}

/**
* Check if the text is underlined
Copy link
Contributor

Choose a reason for hiding this comment

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

underlined or overlined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be changed to "overlined"

/**
* Check if the text is underlined
*
* @return Return the check result
Copy link
Contributor

Choose a reason for hiding this comment

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

check result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be done

/**
* Get the text direction
*
* @return Return the text direction
Copy link
Contributor

Choose a reason for hiding this comment

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

What are the valid values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't checked all at all normaly RTL and LTR

* Check if the text has a hyperlink
*
* @return Return the check result
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

check result?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be done

public boolean isHasHyperlink() {
return hasHyperlink;
}

/**
* Set the text use a hyperlink
Copy link
Contributor

Choose a reason for hiding this comment

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

non descriptive javadoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be changed

Copy link
Contributor

@wimjongman wimjongman left a comment

Choose a reason for hiding this comment

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

Thanks!

@speckyspooky speckyspooky merged commit f8f47f6 into eclipse-birt:master Nov 12, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugFix Change to correct issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants