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

#676: Fix typos in coding-conventions asciidoc #677

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

leonrohne27
Copy link

@leonrohne27 leonrohne27 commented Oct 2, 2024

Fixes: #676

@CLAassistant
Copy link

CLAassistant commented Oct 2, 2024

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link
Collaborator

coveralls commented Oct 2, 2024

Pull Request Test Coverage Report for Build 11176737615

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 66.509%

Totals Coverage Status
Change from base Build 11141097628: 0.0%
Covered Lines: 6126
Relevant Lines: 8868

💛 - Coveralls

Copy link
Contributor

@jan-vcapgemini jan-vcapgemini left a comment

Choose a reason for hiding this comment

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

Thanks for your corrections. I've added some suggestions but overall this looks nice.

@@ -146,10 +146,10 @@ Always use the logger to output messages and never use `System.out` or `System.e
When catching exceptions always ensure the following:

* Never call `printStackTrace()` method on an exception
* Either log or wrap and re-throw the entire catched exception.
* Either log or wrap and re-throw the entire catch exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should still be catched I think, as it is a catched exception.

documentation/coding-conventions.adoc Outdated Show resolved Hide resolved
Be aware that the cause(s) of an exception is very valuable information.
If you loose such information by improper exception-handling you may be unable to properly analyse production problems what can cause severe issues.
** If you wrap and re-throw an exception ensure that the catched exception is passed as cause to the newly created and thrown exception.
** If you wrap and re-throw an exception ensure that the catch exception is passed as cause to the newly created and thrown exception.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should still be catched I think, as it is a catched exception.

@@ -170,7 +170,7 @@ Exception in thread "main" java.lang.IllegalStateException: Something failed
at com.devonfw.tools.ide.ExceptionHandling.main(ExceptionHandling.java:14)
----

As you can see we have no information and clue what the catched `Exception` was and what really went wrong in `doSomething()`.
As you can see we have no information and clue what the catch `Exception` was and what really went wrong in `doSomething()`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should still be catched I think, as it is a catched exception.

@jan-vcapgemini jan-vcapgemini added the documentation Improvements or additions to documentation label Oct 4, 2024
@jan-vcapgemini jan-vcapgemini added this to the release:2024.10.001 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Team Review
Development

Successfully merging this pull request may close these issues.

Fix typos in coding-conventions asciidoc
4 participants