Andras/BoundlessFlowCampus2K

Fork 0

#900-logger-for-middleware #918

Merged

a22erigr merged 3 commits from #900-logger-for-middleware into team_2_week_5

2026-05-13 07:33:45 +00:00

j21marbr commented

2026-05-12 11:24:34 +00:00

Collaborator

Changes:

Replace Console.WriteLine with ILogger
Added information, warning and error logs

Changes: - Replace Console.WriteLine with ILogger - Added information, warning and error logs

j21marbr added 1 commit

2026-05-12 11:24:34 +00:00

Replace Console.WriteLine with ILogger 14e6494653

a22erigr added the

needs review

label

2026-05-12 11:27:54 +00:00

j21marbr added 1 commit

2026-05-12 11:41:52 +00:00

Add ILogger packages to middleware 8cd53231e4

j21marbr commented

2026-05-12 11:46:33 +00:00

Author

Collaborator

No description provided.

bild.png

238 KiB

a22erigr

2026-05-13 06:38:07 +00:00

added the
Under-review
label
requested review from b23albst

b23albst approved these changes

2026-05-13 07:18:25 +00:00

b23albst left a comment

Collaborator

Review on #918

Whats done

Tested that the logs were correctly output and that nothing else was affected

Tested on ubuntu

The middleware container runs like it is supposed to and its logs can be read
I don't know if it was of concern, but the app starts like normal as well

Inspected following files

middleware/middleware/Program.cs
- Indentation/code styling
  - Indentation is ok
- Comments
  - No comments added, but they are not needed in this context, so I think this is fine.
- General code quality
  - Appropriate categories (warning, error, etc.) for the logs
  - The LogError on on line 75 does not follow the same convention as the others. It just logs the exception message. I'm not sure, since I did not get any exceptions, but I think writing the logs like others (logger.LogError(e, ....)) Outputs the stack trace as well as the exception message, which I think is preferable.

What needs to be done

Program.cs line 75
- I think the logger should follow the same convention as the other error logs.

Conclusion

I won't reject the pr, since it is such a small error, and it's just a matter of opinion, but I still think it should be changed.
Other than that, this is totally ready to be merged.

# Review on #918 ## Whats done Tested that the logs were correctly output and that nothing else was affected ### Tested on ubuntu + The middleware container runs like it is supposed to and its logs can be read + I don't know if it was of concern, but the app starts like normal as well ### Inspected following files + middleware/middleware/Program.cs + Indentation/code styling + Indentation is ok + Comments + No comments added, but they are not needed in this context, so I think this is fine. + General code quality + Appropriate categories (warning, error, etc.) for the logs + The LogError on on line 75 does not follow the same convention as the others. It just logs the exception message. I'm not sure, since I did not get any exceptions, but I think writing the logs like others (logger.LogError(e, ....)) Outputs the stack trace as well as the exception message, which I think is preferable. ## What needs to be done + Program.cs line 75 + I think the logger should follow the same convention as the other error logs. ## Conclusion + I won't reject the pr, since it is such a small error, and it's just a matter of opinion, but I still think it should be changed. + Other than that, this is totally ready to be merged.

b23albst added 1 commit

2026-05-13 07:33:05 +00:00

Changed one error log to match the others 5e069a76b2

a22erigr merged commit 7a7fbc81e4 into team_2_week_5

2026-05-13 07:33:45 +00:00

a22erigr referenced this pull request from a commit

2026-05-13 07:33:48 +00:00

Merge pull request '#900-logger-for-middleware' (#918) from #900-logger-for-middleware into team_2_week_5

b25marem referenced this pull request from a commit

2026-05-15 08:56:06 +00:00

Merge pull request '#900-logger-for-middleware' (#918) from #900-logger-for-middleware into team_2_week_5

a22erigr referenced this pull request

2026-05-20 08:23:04 +00:00

Implement logger for Middleware #900

a24sakma commented

2026-05-20 09:58:02 +00:00

Collaborator

This pr needs be reopened as the changes was removed from last week due to issues.

Just make sure you branch is up to date and you re-base with latest changes

This pr needs be reopened as the changes was removed from last week due to issues. Just make sure you branch is up to date and you re-base with latest changes