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 of the FAQ Log entry #1623

Merged

Conversation

vogella
Copy link
Contributor

@vogella vogella commented Nov 20, 2024

Lets promote ILog.get() instead Platform.getLog():

@vogella vogella force-pushed the update_of_faq_log_entry branch from c1a586d to 715490c Compare November 20, 2024 09:03
@vogella
Copy link
Contributor Author

vogella commented Nov 20, 2024

@HannesWell WDTY?

@thahnen FYI, we like to see more PR from you (also code related if you find the time).

@vogella vogella force-pushed the update_of_faq_log_entry branch from 715490c to 7ea6b8b Compare November 20, 2024 09:08
Copy link
Contributor

github-actions bot commented Nov 20, 2024

Test Results

 1 755 files  ±0   1 755 suites  ±0   1h 32m 57s ⏱️ - 5m 58s
 4 170 tests ±0   4 148 ✅ ±0   22 💤 ±0  0 ❌ ±0 
13 107 runs  ±0  12 943 ✅ ±0  164 💤 ±0  0 ❌ ±0 

Results for commit ba30f2d. ± Comparison against base commit 6dd6732.

♻️ This comment has been updated with latest results.

...
public static void test(){
try {
log("Eclipse Style Logging");
ILog.get().info("Starting");
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's not a good idea to use tabs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be resolved

@vogella vogella force-pushed the update_of_faq_log_entry branch from 7ea6b8b to f1c8ca0 Compare November 20, 2024 11:37
try {
log("Eclipse Style Logging");
// do something that may cause an exception
}
catch(Exception e){
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit.

Suggested change
catch(Exception e){
catch (Exception e){

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

Lets promote ILog.get() instead Platform.getLog():
@vogella vogella force-pushed the update_of_faq_log_entry branch from f1c8ca0 to ba30f2d Compare November 20, 2024 11:55
@vogella
Copy link
Contributor Author

vogella commented Nov 20, 2024

Ignoring freeze as this is only online documentation

@vogella vogella merged commit d974aaf into eclipse-platform:master Nov 20, 2024
16 of 17 checks passed
Copy link
Member

@HannesWell HannesWell left a comment

Choose a reason for hiding this comment

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

Thanks. I personally also prefer the new code.

I just wonder if the example should be put in a markdown code block wrapped in three back-ticks.

You can write any kind of IStatus object to the log file, including a MultiStatus if you have hierarchies of information to display. If you create your own subclass of the utility class Status, you can override the getMessage method to return extra information to be displayed in the log file. Many plug-ins add utility classes for writing messages and errors to the log:

import org.eclipse.core.runtime.Status;
import org.eclipse.core.runtime.ILog;
...
public class MyLogger {
...
private static final Bundle BUNDLE = FrameworkUtil.getBundle(MyLogger.class);
Copy link
Member

Choose a reason for hiding this comment

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

Is this line still necessary?
At least while viewing this on the phone I can't see a reference.

@vogella vogella deleted the update_of_faq_log_entry branch November 24, 2024 16:25
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.

3 participants