Resolve "Remove Qt dependencies" - [merged] #96

Closed rainman110 closed 11 months ago

In GitLab by @mariusalexander on Nov 10, 2022, 13:55

Merges 33-remove-qt-dependencies -> master

Closes #33

In GitLab by @mariusalexander on Nov 10, 2022, 16:48

added 1 commit

75b48d82 - initial changes to move away from Qt + cleanup and refactoring

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 10:46

added 1 commit

03174a19 - cleanup, moved destination id to logger::addDestination, removed qt...

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 11:40

added 1 commit

6599bfe7 - code cleanup

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 14:05

added 1 commit

b14b82b4 - refactored gt::log::stream to remove qt dependencies

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 14:21

added 1 commit

c9a85717 - fixed code quality

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 14:30

@rainman110 I've wanted to see if we could remove the Qt dependencie from our logging library. I have refactored most symbols, classes etc. that used Qt. This is still very much WIP but maybe you could have a look if Im heading in the right direction.

I still have to refactor the file destination, as its still using the QFile API. ~Also there is a feature to log in multiple threads (I assume) that still uses Qt.~ I will definetly need tests for both features. I will probably have to look into the streaming class a bit more too, but I wanted to make it work for now. Also I would move the logmodel to GTlab Core once finished.

Some Qt-dependent features may be enabled using GT_LOG_USE_QT_BINDINGS

The rests seems to work fine, I have added more test and a few features I think could prove useful like an optional id for a destination.

In GitLab by @mariusalexander on Nov 11, 2022, 14:35

added 1 commit

38784168 - fixed time buffer on unix

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 15:24

added 2 commits

2cbfa22c - added test for time
dabcc83b - fixd issue in (no)space and (no)quote, added tests, refactored usage of...

Compare with previous version

In GitLab by @mariusalexander on Nov 11, 2022, 16:51

added 1 commit

be97c8b5 - updated readme

Compare with previous version

In GitLab by @mariusalexander on Nov 14, 2022, 07:23

added 2 commits

db4472b8 - cleanup, added more tests
c08bffee - refactoring

Compare with previous version

In GitLab by @mariusalexander on Nov 14, 2022, 08:59

added 1 commit

19277899 - removed separate logging functionality, removed empty files

Compare with previous version

In GitLab by @mariusalexander on Nov 15, 2022, 08:47

added 1 commit

c3dcbe12 - implemented tests for the file rotation null and size strategies

Compare with previous version

In GitLab by @mariusalexander on Nov 15, 2022, 10:15

added 1 commit

43180d87 - removed qt dependencies from file destination sink

Compare with previous version

In GitLab by @mariusalexander on Nov 15, 2022, 10:18

added 1 commit

d5c8a0a5 - fix

Compare with previous version

In GitLab by @mariusalexander on Nov 15, 2022, 10:19

added 1 commit

92bad8be - fix2

Compare with previous version

In GitLab by @mariusalexander on Nov 16, 2022, 10:49

added 1 commit

847136fd - removed final dependecies to qt from source files

Compare with previous version

In GitLab by @mariusalexander on Nov 16, 2022, 11:53

added 1 commit

f9628291 - fix

Compare with previous version

In GitLab by @mariusalexander on Nov 16, 2022, 12:02

@rainman110 I have applied your suggestion to move the qt specific stuff into a separate header file. I've also removed the logmodel.

However because I export a few classes currently i get a bunch of warning in the windows build:

warning C4251: "gt::log::Logger::pimpl": class "std::unique_ptr<gt::log::Logger::Impl,std::default_delete<gt::log::Logger::Impl>>" erfordert eine DLL-Schnittstelle, die von Clients von class "gt::log::Logger" verwendet wird

Am I doing something wrong? I would assume that if we make this library header only we dont have to deal with this, but Im still curious.

In GitLab by @rainman110 on Nov 16, 2022, 12:39

Instead of exporting the whole class, you could alternatively export only the public methods. then, this warning should go away... hopefully

In GitLab by @rainman110 on Nov 16, 2022, 13:19

added 1 commit

4f314856 - Fixed warnings

Compare with previous version

In GitLab by @mariusalexander on Nov 16, 2022, 15:36

added 1 commit

46e91ccf - some refactoring, renamed qt bindings file

Compare with previous version

In GitLab by @mariusalexander on Nov 16, 2022, 15:38

Commented on tests/unittests/test_types.cpp line 3

I have refactored the tests a bit to test that the qt bindings may also be added individually to certain files. It works! :+1:

In GitLab by @mariusalexander on Nov 16, 2022, 15:39

I think this is ready to merge! :) @rainman110

In GitLab by @rainman110 on Nov 16, 2022, 15:40

Commented on tests/unittests/test_types.cpp line 3

Nice :kissing_heart:

In GitLab by @rainman110 on Nov 16, 2022, 16:04

marked this merge request as ready

In GitLab by @rainman110 on Nov 16, 2022, 16:04

In GitLab by @rainman110 on Nov 16, 2022, 19:58

added 1 commit

854f70f9 - Some cleanup test helper

Compare with previous version

In GitLab by @rainman110 on Nov 16, 2022, 20:01

approved this merge request

In GitLab by @rainman110 on Nov 16, 2022, 20:39

unapproved this merge request

In GitLab by @rainman110 on Nov 16, 2022, 20:47

added 1 commit

1501376c - Renamed log to doLog

Compare with previous version

In GitLab by @rainman110 on Nov 16, 2022, 21:00

approved this merge request

In GitLab by @rainman110 on Nov 16, 2022, 21:17

I looked into your code in detail. Awesome work, really!!!

In GitLab by @mariusalexander on Nov 17, 2022, 08:14

added 1 commit

284873c4 - renamed logQuoted to doLogQuoted, logQt to doLogQt

Compare with previous version

In GitLab by @mariusalexander on Nov 17, 2022, 08:17

added 1 commit

f0703e1d - fixed typo

Compare with previous version

In GitLab by @mariusalexander on Nov 17, 2022, 08:17

Awesome, thanks :rocket:

In GitLab by @mariusalexander on Nov 17, 2022, 08:19

@rainman110 I fixed a typo and renamed logQouted to doLogQuoted too. I need your approval once more :white_check_mark:

In GitLab by @rainman110 on Nov 17, 2022, 08:19

approved this merge request

In GitLab by @mariusalexander on Nov 17, 2022, 08:20

enabled an automatic merge when the pipeline for f0703e1df8447822902d17ef2a23f1bd5d261528 succeeds

In GitLab by @mariusalexander on Nov 17, 2022, 09:17

mentioned in commit 1c034aff4155e01d1380a5577a3794bf2df76cc1

dlr-gtlab / gt-logging

Resolve "Remove Qt dependencies" - [merged] #96